Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ENH Pre-render the LinkField in entwine #241

Merged
merged 5 commits into from
Mar 12, 2024
Merged

ENH Pre-render the LinkField in entwine #241

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a891cd
ENH Pre-render the LinkField in entwine
Mar 1, 2024
281a934
MNT Correct disabled link Jest test
Mar 7, 2024
8d0b81f
ENH Move versioned states to a constant
Mar 9, 2024
684dcc5
ENH Use global Spinner template
Mar 9, 2024
26d7050
MNT Revert test change for aria-disabled
Mar 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js
View file
Open in desktop

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions client/src/components/LinkField/LinkField.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,12 @@ 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;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By not skipping the link entries for which we don't have data yet, we'll render a list of placeholder LinkPickerTitle. This will avoids having a big jump on the page once the data finally gets back to us.

}
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}
Expand Down Expand Up @@ -446,7 +446,7 @@ const LinkField = ({

const saveRecordFirst = !loadingError && ownerID === 0;
const renderLoadingError = loadingError;
const renderPicker = !loadingError && !inHistoryViewer && !saveRecordFirst && (isMulti || Object.keys(data).length === 0);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was needed because a disabled LinkField with pe-existing data would briefly render a picker and a place holder link.

With this change, it only renders the picker.

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');
Expand Down
3 changes: 2 additions & 1 deletion client/src/components/LinkField/tests/LinkField-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ test('LinkField will render disabled state if disabled is true', async () => {
const { container } = render(<LinkField {...makeProps({
ownerID: 1,
disabled: true,
value: undefined
emteknetnz marked this conversation as resolved.
Show resolved Hide resolved
})}
/>);
doResolve();
Expand Down Expand Up @@ -176,7 +177,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 () => {
Expand Down
10 changes: 8 additions & 2 deletions lang/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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?'
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/Form/MultiLinkField.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wraps the IDs in an ArrayList so the SS template can loop over them.

}
}
15 changes: 14 additions & 1 deletion templates/SilverStripe/LinkField/Form/LinkField.ss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>

26 changes: 26 additions & 0 deletions templates/SilverStripe/LinkField/Form/LinkField_Spinner.ss
View file
Open in desktop
maxime-rainville marked this conversation as resolved.
Show resolved Hide resolved
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<div class="link-field__loading">
emteknetnz marked this conversation as resolved.
Show resolved Hide resolved
<div class="cms-content-loading-overlay ui-widget-overlay-light"></div>
<div class="cms-content-loading-spinner">
<div class="spinner">
<svg fill="none" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="512" height="297" viewBox="0 0 512 297" class="spinner__animation">
<defs>
<path id="spinner__animation__outline_right_be342372-94f2-4867-a6dd-0f9784c8f4be" d="M253 29L145 105C130 115 126 136 137 150C147 165 168 169 183 159L291 83C335 52 397 63 428 107C459 152 448 214 404 245L370 268C398 316 461 296 490 245C520 191 519 123 482 70C430 -4 327 -22 253 29Z"></path>
<path id="spinner__animation__outline_left_be342372-94f2-4867-a6dd-0f9784c8f4be" d="M258 266L366 191C381 180 385 160 374 145C364 130 343 127 328 137L220 212C176 244 114 233 83 188C52 144 63 82 107 51L141 27C113 -20 50 -0 21 51C-8 104 -7 172 29 226C81 300 184 318 258 266V266Z"></path>
<clipPath id="spinner__animation__mask_right_be342372-94f2-4867-a6dd-0f9784c8f4be">
<use href="#spinner__animation__outline_right_be342372-94f2-4867-a6dd-0f9784c8f4be"></use>
</clipPath>
<clipPath id="spinner__animation__mask_left_be342372-94f2-4867-a6dd-0f9784c8f4be">
<use href="#spinner__animation__outline_left_be342372-94f2-4867-a6dd-0f9784c8f4be"></use>
</clipPath>
</defs>
<use class="spinner__animation__empty" href="#spinner__animation__outline_left_be342372-94f2-4867-a6dd-0f9784c8f4be"></use>
<use class="spinner__animation__empty" href="#spinner__animation__outline_right_be342372-94f2-4867-a6dd-0f9784c8f4be"></use>
<path d="M 379,145 236,242 C 179,282 102,273 62,216 22,159 19,77 76,37 L 135,7" class="spinner__animation__fill-left" clip-path="url(#spinner__animation__mask_left_be342372-94f2-4867-a6dd-0f9784c8f4be)"></path>
<path d="M 138,148 281,50 c 57,-39 129,-30 169,26 39,56 41,136 -14,178 l -47,40" class="spinner__animation__fill-right" clip-path="url(#spinner__animation__mask_right_be342372-94f2-4867-a6dd-0f9784c8f4be)"></path>
<path d="M253 29L145 105C130 115 126 136 137 150C147 165 168 169 183 159L291 83C335 52 397 63 428 107C459 152 448 214 404 245L370 268C398 316 461 296 490 245C520 191 519 123 482 70C430 -4 327 -22 253 29Z"></path>
</svg>
</div>
</div>
</div>


35 changes: 34 additions & 1 deletion templates/SilverStripe/LinkField/Form/MultiLinkField.ss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>

1 change: 1 addition & 0 deletions tests/behat/features/accessibility-linkfield.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
emteknetnz marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
Loading