ENH Pre-render the LinkField in entwine (#241)

* ENH Pre-render the LinkField in entwine

* MNT Correct disabled link Jest test

* ENH Move versioned states to a constant

* ENH Use global Spinner template

* MNT Revert test change for aria-disabled

client/src/components/LinkField/LinkField.js

@@ -27,6 +27,7 @@ import i18n from 'i18n';
 import url from 'url';
 import qs from 'qs';
 import classnames from 'classnames';
+import versionStates from 'constants/versionStates';
 
 export const LinkFieldContext = createContext(null);
 
@@ -267,7 +268,11 @@ const LinkField = ({
    */
   const handleDelete = (linkID, deleteType) => {
     const versionState = data[linkID]?.versionState || '';
-    const isVersioned = ['draft', 'modified', 'published'].includes(versionState);
+    const isVersioned = [
+      versionStates.draft,
+      versionStates.modified,
+      versionStates.published
+    ].includes(versionState);
     const deleteText = isVersioned
       ? i18n._t('LinkField.ARCHIVE_CONFIRM', 'Are you sure you want to archive this link?')
       : i18n._t('LinkField.DELETE_CONFIRM', 'Are you sure you want to delete this link?');
@@ -348,12 +353,14 @@ const LinkField = ({
     const links = [];
     for (let i = 0; i < linkIDs.length; i++) {
       const linkID = linkIDs[i];
-      // Only render items we have data for
-      const linkData = data[linkID];
-      if (!linkData) {
+      // Render dataless item to provide a good loading experience, except if we have single link field
+      const linkData = data[linkID] || {};
+      if (!linkData && !isMulti) {
         continue;
       }
-      const type = types.hasOwnProperty(linkData.typeKey) ? types[linkData.typeKey] : {};
+      const type = types.hasOwnProperty(linkData.typeKey) ?
+        types[linkData.typeKey] :
+        {icon: 'font-icon-link' };
       links.push(<LinkPickerTitle
         key={linkID}
         id={linkID}
@@ -446,7 +453,7 @@ const LinkField = ({
 
   const saveRecordFirst = !loadingError && ownerID === 0;
   const renderLoadingError = loadingError;
-  const renderPicker = !loadingError && !inHistoryViewer && !saveRecordFirst && (isMulti || Object.keys(data).length === 0);
+  const renderPicker = !loadingError && !inHistoryViewer && !saveRecordFirst && (isMulti || linkIDs.length === 0);
   const renderModal = !loadingError && !saveRecordFirst && Boolean(editingID);
   const loadingErrorText = i18n._t('LinkField.FAILED_TO_LOAD_LINKS', 'Failed to load link(s)');
   const saveRecordFirstText = i18n._t('LinkField.SAVE_RECORD_FIRST', 'Cannot add links until the record has been saved');

client/src/components/LinkField/tests/LinkField-test.js

@@ -86,12 +86,30 @@ test('LinkField returns list of links if they exist', async () => {
 });
 
 test('LinkField will render disabled state if disabled is true', async () => {
+  const { container } = render(<LinkField {...makeProps({
+    ownerID: 1,
+    disabled: true
+  })}
+  />);
+  await doResolve({ json: () => ({
+    123: {
+      title: 'Page title',
+      typeKey: 'mylink',
+    }
+  }) });
+  await screen.findByText('Page title');
+  const linkPicker = container.querySelectorAll('#link-picker__link-123');
+  expect(linkPicker[0]).toHaveAttribute('aria-disabled');
+  expect(linkPicker[0]).toHaveClass('link-picker__link--disabled');
+});
+
+test('Empty disabled LinkField will display cannot edit message', async () => {
   const { container } = render(<LinkField {...makeProps({
     ownerID: 1,
     disabled: true,
+    value: undefined
   })}
   />);
-  doResolve();
   await screen.findByText('Cannot create link');
   expect(container.querySelectorAll('.link-picker')).toHaveLength(1);
   expect(container.querySelectorAll('.link-picker')[0]).toHaveTextContent('Cannot create link');
@@ -176,7 +194,7 @@ test('LinkField will render loading indicator if ownerID is not 0', async () =>
   />);
   expect(container.querySelectorAll('.link-field__save-record-first')).toHaveLength(0);
   expect(container.querySelectorAll('.link-field__loading')).toHaveLength(1);
-  expect(container.querySelectorAll('.link-picker')).toHaveLength(1);
+  expect(container.querySelectorAll('.link-picker')).toHaveLength(0);
 });
 
 test('LinkField will render link-picker if ownerID is not 0 and isMulti and has finished loading', async () => {

client/src/components/LinkPicker/LinkPickerTitle.js

@@ -7,6 +7,7 @@ import { LinkFieldContext } from 'components/LinkField/LinkField';
 import { Button } from 'reactstrap';
 import { useSortable } from '@dnd-kit/sortable';
 import { CSS } from '@dnd-kit/utilities';
+import versionStates from 'constants/versionStates';
 
 const stopPropagation = (fn) => (e) => {
   e.nativeEvent.stopImmediatePropagation();
@@ -19,10 +20,10 @@ const stopPropagation = (fn) => (e) => {
 const getVersionedBadge = (versionState) => {
   let title = '';
   let label = ''
-  if (versionState === 'draft') {
+  if (versionState === versionStates.draft) {
     title = i18n._t('LinkField.LINK_DRAFT_TITLE', 'Link has draft changes');
     label = i18n._t('LinkField.LINK_DRAFT_LABEL', 'Draft');
-  } else if (versionState === 'modified') {
+  } else if (versionState === versionStates.modified) {
     title = i18n._t('LinkField.LINK_MODIFIED_TITLE', 'Link has unpublished changes');
     label = i18n._t('LinkField.LINK_MODIFIED_LABEL', 'Modified');
   } else {
@@ -108,11 +109,11 @@ const LinkPickerTitle = ({
     classes[`link-picker__link--${versionState}`] = true;
   }
   const className = classnames(classes);
-  const deleteText = ['unversioned', 'unsaved'].includes(versionState)
+  const deleteText = [versionStates.unversioned, versionStates.unsaved].includes(versionState)
     ? i18n._t('LinkField.DELETE', 'Delete')
     : i18n._t('LinkField.ARCHIVE', 'Archive');
   const ariaLabel = i18n._t('LinkField.EDIT_LINK', 'Edit link');
-  if (['draft', 'modified'].includes(versionState)) {
+  if ([versionStates.draft, versionStates.modified].includes(versionState)) {
     onUnpublishedVersionedState();
   }
   // Remove the default tabindex="0" attribute from the sortable element because we're going to manually
@@ -155,10 +156,12 @@ const LinkPickerTitle = ({
           <span className="link-picker__title-text">{title}</span>
           {getVersionedBadge(versionState)}
         </div>
-        <small className="link-picker__type">
-          {typeTitle}:&nbsp;
-          <span className="link-picker__url">{description}</span>
-        </small>
+        {typeTitle && (
+          <small className="link-picker__type">
+            {typeTitle}:&nbsp;
+            <span className="link-picker__url">{description}</span>
+          </small>
+        )}
       </div>
       {(canDelete && !readonly && !disabled) &&
         // This is a <span> rather than a <Button> because we're inside a <Button> and
@@ -180,7 +183,7 @@ LinkPickerTitle.propTypes = {
   id: PropTypes.number.isRequired,
   title: PropTypes.string,
   description: PropTypes.string,
-  versionState: PropTypes.string,
+  versionState: PropTypes.oneOf(Object.values(versionStates)),
   typeTitle: PropTypes.string.isRequired,
   typeIcon: PropTypes.string.isRequired,
   onDelete: PropTypes.func.isRequired,
@@ -198,4 +201,8 @@ LinkPickerTitle.propTypes = {
   buttonRef: PropTypes.object.isRequired,
 };
 
+LinkPickerTitle.defaultProps = {
+  versionState: versionStates.unversioned,
+}
+
 export default LinkPickerTitle;

client/src/constants/versionStates.js

@@ -0,0 +1,9 @@
+const versionStates = {
+  draft: 'draft',
+  modified: 'modified',
+  unversioned: 'unversioned',
+  unsaved: 'unsaved',
+  published: 'published',
+};
+
+export default versionStates;

composer.json

@@ -17,7 +17,8 @@
         "php": "^8.1",
         "silverstripe/framework": "^5.2",
         "silverstripe/cms": "^5",
-        "silverstripe/versioned": "^2"
+        "silverstripe/versioned": "^2",
+        "silverstripe/admin": "^2.2"
     },
     "require-dev": {
         "silverstripe/frameworktest": "^1",

lang/en.yml

@@ -3,6 +3,8 @@ en:
     CREATE_LINK: 'Create link'
     MENUTITLE: 'Link fields'
     UPDATE_LINK: 'Update link'
+  SilverStripe\LinkField\Form\AbstractLinkField:
+    INVALID_TYPECLASS_EMPTY: '"{class}": Allowed types cannot be empty'
   SilverStripe\LinkField\Form\Traits\AllowedLinkClassesTrait:
     INVALID_TYPECLASS: '"{class}": {typeclass} is not a valid Link Type'
     INVALID_TYPECLASS_EMPTY: '"{class}": Allowed types cannot be empty'
@@ -38,8 +40,8 @@ en:
     SINGULARNAME: 'File Link'
     has_one_File: File
   SilverStripe\LinkField\Models\Link:
-    LINK_TEXT_TITLE: 'Link text'
     LINK_TEXT_TEXT_DESCRIPTION: 'If left blank, an appropriate default will be used on the front-end'
+    LINK_TEXT_TITLE: 'Link text'
     LINK_TYPE_TITLE: 'Link Type'
     MISSING_DEFAULT_TITLE: '(No value provided)'
     OPEN_IN_NEW_TITLE: 'Open in new window?'
@@ -48,10 +50,14 @@ en:
       one: 'A Link'
       other: '{count} Links'
     SINGULARNAME: Link
-    db_OpenInNew: 'Open in new'
     db_LinkText: 'Link text'
+    db_OpenInNew: 'Open in new'
+    db_Sort: Sort
     db_Version: Version
     has_one_Owner: Owner
+  SilverStripe\LinkField\Models\MultiLinkField:
+    AddLink: 'Add link'
+    EditLink: 'Edit link'
   SilverStripe\LinkField\Models\PhoneLink:
     LINKLABEL: 'Phone number'
     PHONE_FIELD: Phone

src/Form/MultiLinkField.php

@@ -3,6 +3,7 @@
 namespace SilverStripe\LinkField\Form;
 
 use LogicException;
+use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\Relation;
 use SilverStripe\ORM\SS_List;
@@ -132,4 +133,9 @@ private function loadFrom(DataObject $record): void
         $value = array_values($relation->getIDList() ?? []);
         parent::setValue($value);
     }
+
+    public function LinkIDs(): ArrayList
+    {
+        return ArrayList::create($this->dataValue());
+    }
 }

templates/SilverStripe/LinkField/Form/LinkField.ss

@@ -1,2 +1,15 @@
+<%-- This template is here to bootstrap a LinkField React form field --%>
+<%-- It includes some pre-rendered content to provide a nicer UI while waiting for React to boot --%>
+<%-- Once React is done pre-rendering, it will discard the pre-rendered markup --%>
 <input $AttributesHTML />
-<div data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProp"></div>
+<div data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProp">
+  <div class="link-field__container">
+    <% include SilverStripe/LinkField/Form/LinkField_Spinner  %>
+    <div>
+        <div class="link-picker__link link-picker__link--is-first link-picker__link--is-last form-control link-picker__link--disabled link-picker__link--published" role="button" aria-disabled="false" aria-roledescription="sortable" aria-describedby="" id="link-picker__link-42">
+            <button type="button" disabled="" class="link-picker__button font-icon-link btn btn-secondary disabled"></button>
+        </div>
+    </div>
+  </div>
+</div>
+

templates/SilverStripe/LinkField/Form/LinkField_Spinner.ss

@@ -0,0 +1,9 @@
+<%-- This template is here to bootstrap a LinkField React form field --%>
+<%-- It includes a pre-rendered spinner to provide a nicer UI while waiting for React to boot --%>
+<%-- Once React is done pre-rendering, it will discard the pre-rendered markup --%>
+<div class="link-field__loading">
+    <div class="cms-content-loading-overlay ui-widget-overlay-light"></div>
+    <% include SilverStripe/Admin/Includes/CMSLoadingSpinner  %>
+</div>
+
+

templates/SilverStripe/LinkField/Form/MultiLinkField.ss

@@ -1,2 +1,35 @@
+<%-- This template is here to bootstrap a MultiLinkField React form field --%>
+<%-- It includes some pre-rendered content to provide a nicer UI while waiting for React to boot --%>
+<%-- Once React is done pre-rendering, it will discard the pre-rendered markup --%>
 <input $AttributesHTML />
-<div data-is-multi="true" data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProp"></div>
+<div data-is-multi="true" data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProp">
+
+  <div class="link-field__container">
+    <% include SilverStripe/LinkField/Form/LinkField_Spinner  %>
+    <div class="link-picker form-control">
+      <div class="link-picker__menu dropdown">
+        <button
+          type="button"
+          aria-haspopup="true"
+          aria-expanded="false"
+          class="link-picker__menu-toggle font-icon-plus-1 dropdown-toggle btn btn-secondary"
+          aria-label="<%t SilverStripe\LinkField\Models\MultiLinkField.AddLink "Add link" %>">
+            <%t SilverStripe\LinkField\Models\MultiLinkField.AddLink "Add link" %>
+        </button>
+      </div>
+    </div>
+    <div class="link-picker-links">
+      <% loop $LinkIDs %>
+      <div class="link-picker__link form-control link-picker__link--published <% if $IsFirst %>link-picker__link--is-first<% end_if %> <% if $IsLast %>link-picker__link--is-last<% end_if %>">
+        <button
+          type="button"
+          disabled=""
+          class="link-picker__button font-icon-link btn btn-secondary disabled"
+          aria-label="<%t SilverStripe\LinkField\Models\MultiLinkField.EditLink "Edit link" %>"></button>
+      </div>
+      <% end_loop %>
+    </div>
+  </div>
+
+</div>
+

tests/behat/features/accessibility-linkfield.feature

@@ -96,6 +96,7 @@ Feature: Accessibility Tests
 
     Then I press the "Tab" key globally
     And I press the "Tab" key globally
+    And I press the "Tab" key globally
     And I press the "Enter" key globally
     And I should see "Page on this site" in the "[data-field-id='Form_EditForm_HasManyLinks'] .dropdown-menu.show" element
     And I press the "Down" key globally