API Making the read-only version of TextCheckboxGroupField more readable

- TextCheckboxGroupField now scaffolds its own fields. The read-only and react implementations expect specifically 2 fields in that particular order.
- Fixing the class attribute -text-checkout-group-field, +text-checkbox-group-field
- Converting the fields to literals when readonly and updating the styles so it looks good
- Moving the read-only namespacing of fields to occur after transformations so that it's easier to refer to fields by their names

client/src/components/TextCheckboxGroupField/TextCheckboxGroupField.js

@@ -5,19 +5,29 @@ import fieldHolder from 'components/FieldHolder/FieldHolder';
 const TextCheckboxGroupField = (props) => {
   const { children } = props;
 
+  // Map out the children and clone to set the "noHolder" prop on them.
   const childrenWithProps = React.Children.toArray(
     React.Children.map(children, child =>
       React.cloneElement(child, { noHolder: true })
     )
   );
 
+  // If readonly we'll just show two literal fields next to each other
+  if (props.readOnly) {
+    return (
+      <div className="text-checkbox-group-field--read-only">
+        {childrenWithProps}
+      </div>
+    );
+  }
+
   // If the checkbox has been removed, just render the TextField on its own
   if (childrenWithProps.length === 1) {
     return childrenWithProps[0];
   }
 
   return (
-    <InputGroup className="text-checkout-group-field">
+    <InputGroup className="text-checkbox-group-field">
       {childrenWithProps[0]}
       <InputGroupAddon addonType="append">
         <InputGroupText>{childrenWithProps[1]}</InputGroupText>

client/src/components/TextCheckboxGroupField/TextCheckboxGroupField.scss

@@ -1,9 +1,21 @@
 // Temporarily overwriting the styles from the CheckboxField component
 // since it cannot be removed easily
-.text-checkout-group-field {
+.text-checkbox-group-field {
   .form-check {
     display: inline;
     padding-left: 0;
     margin-bottom: 0;
   }
+
+  &--read-only {
+    display: flex;
+
+    :first-child {
+      flex: 1;
+    }
+
+    .show-title {
+      font-style: italic;
+    }
+  }
 }

src/Forms/ElementalAreaField.php

@@ -13,6 +13,7 @@
 use SilverStripe\Forms\CompositeField;
 use SilverStripe\Forms\FieldGroup;
 use SilverStripe\Forms\FieldList;
+use SilverStripe\Forms\FormField;
 use SilverStripe\Forms\GridField\GridField;
 use SilverStripe\Forms\TabSet;
 use SilverStripe\ORM\DataObjectInterface;
@@ -148,11 +149,6 @@ protected function getReadOnlyBlockReducer()
             // Set values (before names don't match anymore)
             $elementFields->setValues($element->getQueriedDatabaseFields());
 
-            // Ensure field names are unique between elements on parent form
-            $elementFields->recursiveWalk(function ($field) use ($parentName) {
-                $field->setName($parentName . '_' . $field->getName());
-            });
-
             // Combine into an appropriately named group
             $elementGroup = FieldGroup::create($elementFields);
             $elementGroup->setForm($this->getForm());
@@ -192,6 +188,17 @@ public function performReadonlyTransformation()
         );
 
         $readOnlyField = $readOnlyField->performReadonlyTransformation();
+
+        // Ensure field names are unique between elements on parent form but only after transformations have been
+        // performed
+        /** @var FieldGroup $elementForm */
+        foreach ($readOnlyField->getChildren() as $elementForm) {
+            $parentName = $elementForm->getName();
+            $elementForm->getChildren()->recursiveWalk(function (FormField $field) use ($parentName) {
+                $field->setName($parentName . '_' . $field->getName());
+            });
+        }
+
         return $readOnlyField
             ->setReadOnly(true)
             ->setName($this->getName())

src/Forms/TextCheckboxGroupField.php

@@ -2,7 +2,10 @@
 
 namespace DNADesign\Elemental\Forms;
 
+use SilverStripe\Forms\CheckboxField;
 use SilverStripe\Forms\CompositeField;
+use SilverStripe\Forms\LiteralField;
+use SilverStripe\Forms\TextField;
 
 class TextCheckboxGroupField extends CompositeField
 {
@@ -11,13 +14,22 @@ class TextCheckboxGroupField extends CompositeField
     /**
      * Set the composite's title to that of the first child
      *
-     * {@inheritDoc}
+     * @param string|null $title
      */
-    public function __construct(...$children)
+    public function __construct($title = null)
     {
-        parent::__construct($children);
+        if (!$title) {
+            $title = _t(__CLASS__ . '.TitleLabel', 'Title (displayed if checked)');
+        }
+
+        $fields = [
+            TextField::create('Title', $title),
+            CheckboxField::create('ShowTitle', _t(__CLASS__ . '.ShowTitleLabel', 'Displayed'))
+        ];
+
+        parent::__construct($fields);
 
-        $this->setTitle($this->getChildren()->first()->Title());
+        $this->setTitle($title);
     }
 
     /**
@@ -30,7 +42,16 @@ public function performReadonlyTransformation()
         $field = parent::performReadonlyTransformation();
 
         $field->setTemplate(CompositeField::class);
-        $field->setTitle(null);
+        $field->setTitle('Title');
+
+        $field->replaceField('Title', LiteralField::create(
+            'Title',
+            $field->fieldByName('Title')->Value()
+        ));
+        $field->replaceField('ShowTitle', LiteralField::create(
+            'ShowTitle',
+            $field->fieldByName('ShowTitle')->Value() ? 'Displayed' : 'Not displayed'
+        )->addExtraClass('show-title'));
 
         return $field;
     }

src/Models/BaseElement.php

@@ -299,10 +299,7 @@ public function getCMSFields()
             $fields->removeByName('ShowTitle');
             $fields->replaceField(
                 'Title',
-                TextCheckboxGroupField::create(
-                    TextField::create('Title', _t(__CLASS__ . '.TitleLabel', 'Title (displayed if checked)')),
-                    CheckboxField::create('ShowTitle', _t(__CLASS__ . '.ShowTitleLabel', 'Displayed'))
-                )
+                TextCheckboxGroupField::create()
                     ->setName('Title')
             );
 

src/Models/ElementalArea.php

@@ -13,7 +13,6 @@
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\ORM\FieldType\DBHTMLText;
-use SilverStripe\ORM\HasManyList;
 use SilverStripe\ORM\UnsavedRelationList;
 use SilverStripe\Versioned\Versioned;
 
@@ -22,8 +21,6 @@
  * @package DNADesign\Elemental\Models
  *
  * @property string $OwnerClassName
- *
- * @method HasManyList|BaseElement[] Elements()
  */
 class ElementalArea extends DataObject
 {

tests/Forms/TextCheckboxGroupFieldTest.php

@@ -19,15 +19,13 @@ protected function setUp()
     {
         parent::setUp();
 
-        $this->field = new TextCheckboxGroupField(
-            new TextField('HelloWorld'),
-            new CheckboxField('Display')
-        );
+        $this->field = new TextCheckboxGroupField('Title');
     }
 
     public function testFieldIsAssignedFirstFieldsTitleInConstructor()
     {
-        $this->assertSame('Hello World', $this->field->Title());
+        $this->assertSame('Title', $this->field->Title());
+        $this->assertSame('Title', $this->field->getChildren()->first()->Title());
     }
 
     public function testFieldReturnsCompositeFieldTemplateOnReadonlyTransformation()