Follow-up to #2914484: Prevent the layout field from being removed if overrides exist
When attempting to disable Layout Builder for a display, a confusing "Your settings have been saved" message is shown on the Confirmation form before the submit button has been pressed.
Install a fresh standard site
Enable the LB module
Add Layout Builder to an entity display
Disable Layout Builder on the display
Verify on the confirmation form that a confusing "Your settings have been saved" message is shown. See #10
Remove this message from the confirmation form.
Track down the cause of this messsage and remove it from the confirmation form.
The message will no longer show on the confirm form when disabling Layout Builder on a display.
Before
After
N/A
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | Screenshot 2024-04-17 at 11.01.03 AM.png | 96.29 KB | smustgrave |
| #26 | Screenshot 2024-04-17 at 11.01.22 AM.png | 141.54 KB | smustgrave |
| #10 | hope_not.png | 107.67 KB | xjm |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !7489
Comments
Comment #2
tim.plunkettComment #3
dyannenovaI think the most straightforward way to handle this would be description text below the grayed out checkbox that explains that there are content layouts saved with this setting, and that the content layouts will need to be reset before the setting can be changed, while linking that text to a batch removal confirmation page.
It's not the easiest way for the user to get the setting changed, but I think it's the most clear.
Comment #4
michaelfavia commentedComment #5
kristen pol@DyanneNova Could you mark up some screenshots where the message would show up?
Also, if someone added steps to reproduce the bad UX, that would be helpful. Thanks.
Comment #6
tim.plunkettComment #8
tim.plunkettComment #9
tim.plunkettThe parent issue has the stable blocker tag.
Comment #10
xjmI can't find where I originally reported the "for example" bug in the summary, but I think it means this?
The issue title doesn't make that clear; there's an actual UX bug with the (incorrect, but alarming) message that needs to be fixed. Should we file that as a child issue, or retitle this?
Comment #12
tim.plunkettComment #21
danielvezaVerified from manual testing that this is still an issue. Updating the IS and added steps to replicate.
Comment #23
danielvezaOpened a MR to stop the message from showing on the Disable Layout Builder Form.
There was no specific test coverage for the disable form itself, rather than add a whole new test for this I added coverage to an existing test that was already asserting LB messages.
Comment #24
danielveza@mstrelan raised a clever alternate approach on the MR. Setting back to NW for that
Comment #25
danielvezaUpdated the approach based on the feedback from @mstrelan
Comment #26
smustgrave commentedCan confirm I'm seeing the issue described following the steps of the issue summary.
Attaching before/after screenshots to the summary.
MR does fix the issue.
Comment #27
longwaveNot sure this will work if the string has been translated or altered via custom strings; is there no way of preventing the message from being added in the first place?
Comment #28
danielvezaBased on @longwave's comment in #27 I've built a POC of an alternate approach that prevents the message from being added to the messenger().
This is a more generic approach that other forms could use in the future if this comes up again in other places in core.
Comment #29
smustgrave commentedCan confirm still working @longwave what do you think?
Comment #30
smustgrave commentedTo keep this one from stalling going to mark but new solution is also still working as mentioned. If I jumped the gun apologize
Comment #31
longwaveSimplified this even further, if we redirect at all then we shouldn't show the message?
Having said that, the message is partially correct; other settings on the form *have* been saved by this point.
Comment #32
smustgrave commentedNew approach appears to work as well.
Comment #33
danielvezaI was concerned the new approach could stop the message from showing when we do want it to display, but I've checked all child classes of
EntityDisplayFormBaseand the only one that sets redirects in the form state isLayoutBuilderEntityViewDisplayForm.I prefer the approach taken here to my original MR & it's flexible enough to be used in the future if something like this pops up again. So +1 from me
Comment #38
catchCommitted/pushed to 11.x and cherry-picked back through to 10.3.x, thanks!