sunnysideup / silverstripe-cms-niceties

src/Traits/CMSNicetiesTraitForTabs.php

<?php

namespace Sunnysideup\CMSNiceties\Traits;

use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Tab;
use SilverStripe\Versioned\Versioned;

trait CMSNicetiesTraitForTabs
{
    public function addSeparator(FieldList $fields, string $name, ?string $after = 'Main')
    {
        if (false !== $after) {

The condition false !== $after is always true.

            $tab = Tab::create($name, '|');
            $fields->insertAfter($after, $tab);
        } else {
            $fields->addFieldsToTab(
                'Root.' . $name,
                []
            );
            $fields->fieldByName('Root.' . $name)->setTitle('|');
        }
    }

    public function reorderTabs(FieldList $fields, array $tabOrder): FieldList
    {
        $tabs = [];
        foreach ($tabOrder as $tabName => $title) {
            // non-associative array..
            if ((int) $tabName === $tabName) {
                $tabName = $title;
                $items = preg_split('#(?=[A-Z])#', $tabName);
                $title = is_array($items) ? trim(implode(' ', $items)) : $tabName;

The assignment to $title is dead and can be removed.

            }

            $tabNamePlus = $tabName . 'Tab';

            // fixd existing existing tab
            $tab = $fields->fieldByName('Root.' . $tabName);

Are you sure the assignment to $tab is correct as $fields->fieldByName('Root.' . $tabName) targeting SilverStripe\Forms\FieldList::fieldByName() seems to always return null.

            if (! $tab) {
                $tab = $fields->fieldByName('Root.' . $tabNamePlus);

Are you sure the assignment to $tab is correct as $fields->fieldByName('Root.' . $tabNamePlus) targeting SilverStripe\Forms\FieldList::fieldByName() seems to always return null.

            }

            if (! $tab) {
                $tab = new Tab($tabNamePlus, $tabName);
            }

            $fields->removeByName(['Root.' . $tabName]);
            $fields->removeByName(['Root.' . $tabNamePlus]);
            $fields->removeFieldFromTab('Root', $tabName);
            $fields->removeFieldFromTab('Root', $tabNamePlus);
            $fields->removeFieldsFromTab('Root', [$tabName]);
            $fields->removeFieldsFromTab('Root', [$tabNamePlus]);
            $tab->setTitle($tabName);
            $tab->setName($tabNamePlus);
            $tabs[] = $tab;
            // $fields->addFieldsToTab('Root', $tab);
        }

        // $tabs = array_reverse($tabs);
        foreach ($tabs as $tab) {
            $fields->addFieldToTab('Root', $tab);
        }

        return $fields;
    }

    public function addTab(FieldList $fields, string $name, ?string $after = 'Main', ?string $title = '')
    {
        // add spaces between capitals
        if (! $title) {
            $items = preg_split('#(?=[A-Z])#', $name);
            $title = is_array($items) ? trim(implode(' ', $items)) : $name;

The condition is_array($items) is always true.

        }

        if (null !== $after) {
            if ($this->hasExtension(Versioned::class) && $this->isArchived()) {

It seems like isArchived() must be provided by classes using this trait. How about adding it as abstract method to this trait?

It seems like hasExtension() must be provided by classes using this trait. How about adding it as abstract method to this trait?

                // do nothing
            } else {
                $tab = $fields->fieldByName('Root.' . $name);

Are you sure the assignment to $tab is correct as $fields->fieldByName('Root.' . $name) targeting SilverStripe\Forms\FieldList::fieldByName() seems to always return null.

                $fields->removeFieldFromTab(
                    'Root',
                    $name
                );
                if (! $tab) {

$tab is of type null, thus it always evaluated to false.

                    $tab = Tab::create($name, $title);
                }

                $fields->insertAfter($after, $tab);
            }
        } else {
            $fields->addFieldsToTab(
                'Root.' . $name,
                []
            );
            $fields->fieldByName('Root.' . $name)->setTitle($title);

Are you sure the usage of $fields->fieldByName('Root.' . $name) targeting SilverStripe\Forms\FieldList::fieldByName() seems to always return null.

        }
    }
}