ENH Improved validation

[emteknetnz]

Issue #17

Requires silverstripe/silverstripe-framework#11116 to be merged first before CI works

[purplespider]

For the ExternalLink field validation etc, can I suggest considering using https://github.com/burnbright/silverstripe-externalurlfield? (I do feel that module should really be part of the core, as it would help keep external links and their fields consistent between different modules).

[GuySartorelli]

Looks good, will merge after framework PR.

[GuySartorelli]

Works great on a page.

When I try in an inline-elemental block, the first time I try to create a link and trigger validation (i.e. an invalid link), the modal just closes and reopens.
Submitting again triggers validation and gives the message correctly, so it's just the first time submitting the modal form with an invalid link.

[emteknetnz]

When I try in an inline-elemental block, the first time I try to create a link and trigger validation (i.e. an invalid link), the modal just closes and reopens. Submitting again triggers validation and gives the message correctly, so it's just the first time submitting the modal form with an invalid link.

I've spent half a day debugging this. This is seems like an extremely difficult issue to fix. Core issue is that the ElementList.js (possibly higher up) is re-rendering on validation failure, causing the nested LinkField and nested FormBuilderModal to themselves re-render.

LinkFields on Page don't have this issue because they're not nested inside a react component higher up that keeps re-rendering

This can be validated by putting in the following into (silverstripe/silverstripe-elemental) ElementEditor.js and putting breakpoints in and causing the validation to fail - componentDidUpdate() will trigger on the 1st validation failure. It will not trigger on the 2nd validation failure.

  componentDidMount() {
    var x = 1;
  }

  componentDidUpdate() {
    var x = 1;
  }

What's causing the re-render seems like its in the "magical blackbox" territory of JS Injector which does a bunch of stuff in registerTransforms.js and possibly redux-form. I don't really know. I've pretty confident that if we're able to get a fix for this it won't be in linkfield, it'll likely be in elemental though possibly admin instead

There's an outside chance there's some cross-over with silverstripe/silverstripe-admin#1662,

Debugging in entwine.js on root.render(<ElementEditorComponent {...props} />); this does NOT trigger a breakpoint the same way as other things do, so ElementEditor.js is the top-most bit

[emteknetnz]

Update: Max traced it to this line - making it so that Injector isn't used fixes the issue and instead we just use <LinkModal> as per normal fixes issue

I've leave this code as is in this PR and look to fix or remove on this card

[GuySartorelli]

For my money, passing the injected modal as a prop or memoising it seems likely to resolve this.

[emteknetnz]

I had a quick try with

// https://react.dev/reference/react/memo
import React, { memo } from 'react';

const LinkModal = memo(loadComponent(`LinkModal.${handlerName}`));

Didn't work

Passing in a prop (haven't tested) is kinda ugly because <LinkModalContainer> can come from either LinkField.js or LinkPicker.js

Passing as a prop from LinkPicker.js to LinkModalContainer.js didn't seem to work either

[GuySartorelli]

Max came up with a solution. I've simplied it a lot, the only part that's really needed is setting the loadComponent result as state like so:

import React, { useState } from 'react';

// ...
const [ LinkModal, setLinkModal ] = useState(null);

// ...
if (LinkModal === null) {
  setLinkModal(() => loadComponent(`LinkModal.${handlerName}`));
}

You can wrap setting the state in a call to useEffect as well if you like (using [typeKey] as the second argument) - but in my (admittedly limited) test that didn't seem necessary.

[GuySartorelli]

Works great locally, just a tiny docs regression

[GuySartorelli]

LGTM