Problem/Motivation

In #3528205: Performance: On-demand lazy-loading source selector & #3478643: UX of source selector for slots, we focused on asynchronous load for better performance (no need to create a plugin instance of each source when we are not using the selector).

But we also introduced some UX changes:

  • some already discussed for long and warmly welcomed, like displaying only the default source form when it is a "widget" or the settings summary
  • some with more improvisation, so to be challenged, like the modal system

We need to address those UX topics before releasing 2.0.22.

Proposed resolution

Let's talk about...

1. Replace details element by a simple "flow" of props ?

Moved to its own issue: #3622982: Remove details element in component form?

2. Open the AJAX inline instead of in modal?

Also discussed in #3622509: Let hosts open the source selector inline instead of in a dialog.

Can we keep the full AJAX logic but load the modal content inside the prop form? As a replacement of the widget form, hiding the "settings" button while we are configuring the sources:
ia

If we keep the settings button in the details summary, let's not forget this in order to avoid some misalignment:

:is(.uip-prop-box, 
#drupal-off-canvas-wrapper .uip-prop-box) summary .uip-source-selector-toggle--open {
  height: auto;
}

3. How to format PluginSettingsInterface::settingsSummary() ?

(Investigation in progress)

What about slot forms?

Those proposals are bout prop forms, but we have also moved to asynchronous loading for slots. Are those proposals also applicable for slot forms? Is there something to be careful about?

For information, Display Builder don't use the slot forms, so it may be addressed in a later follow-up.

CommentFileSizeAuthor
clean.png44.32 KBpdureau
detail-noon-collapsable.png48.18 KBpdureau
inlineajax.png68 KBpdureau

Issue fork ui_patterns-3622928

Command icon Show commands

Start 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:

About issue forks

Comments

pdureau created an issue. See original summary.

just_like_good_vibes made their first commit to this issue’s fork.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: Unassigned » just_like_good_vibes

just_like_good_vibes opened merge request !564

pdureau’s picture

pdureau commented
Issue summary: View changes
pdureau’s picture

pdureau commented
Status: Active » Needs work

Review of the current state of the MR. Commit: 13d47b60e8b1c4b7c774695a9f5a88205262efe5

1. Replace details element by a simple "flow" of props ?

Moved to its own issue: #3622982: Remove details element in component form?

2. Open the AJAX inline instead of in modal?

UI feedback

We lost the little "settings" icon in the details summary. It is cool to have a (way) simpler JS & CSS code, and we may decide it is OK like that.

But this icon has 3 advantages:

  • it tells users which props have more available sources according to context
  • it is smaller (less vertical space taken) and cleaner looking
  • if it disappears when in inline modal, it gives a clear indication of what is going on

Can we restore it with not too much code?

Test with UI Patterns Field Formatters

✅ OK, works well.

Test with UI Patterns Layout Builder

✅ OK, works well. Also with blocks embedding field formatters (the tricky use case from #3528205: Performance: On-demand lazy-loading source selector)

Test with Display Builder

Tested with #3622023: Behaviors are not attached on htmx swaps outside the builder root (side panels).

⚠️ I got an a new page instead of a the inline AJAX once, but it may be a cache issue from before I switch to the new branch. Let's be careful anyway.

❌ I need to explicitly click on "Update" button (the one we may remove #3554416: Remove Update button from ContextualFormPanel before 1.0.0).

To reproduce:

  1. drop a SDC with an URL prop
  2. in the contextual form panel, click "Configure" to get the source selector of this URL prop
  3. select "Token" source, put "[site:login-url]"
  4. ❌ the value is not sent to Display Builder, we knwow that because the URL is not active in the Preview panel
  5. click on "Update", the value is sent to Display Builder

It is a bit tricky, because it works sometimes, maybe the first time only. It is hard to identify the exact cause.

Test with UI Patterns UI

To check.

3. How to format PluginSettingsInterface::settingsSummary() ?

Not done in the MR yet, because we didn't define the expectations yet.

pdureau’s picture

pdureau commented
Issue summary: View changes
mogtofu33’s picture

mogtofu33 commented

Feedback from Display Builder, tested with !564 at 757705b8, in the Config drawer (about 270px wide), with UI Suite Bootstrap as the front theme.

About the Display Builder results in #6

The full page reloads, the need to click Update, and the inconsistency were one cause, now fixed in Display Builder 1.0.x #3622023: Behaviors are not attached on htmx swaps outside the builder root (side panels). On core 11.x, the htmx asset loader attaches behaviors before the scripts it just loaded have run. So the first component form's #ajax buttons were not bound, and they submitted the page. Later forms worked because the script was already there. Display Builder now loads core/drupal.ajax up front. Core's main branch fixes the loader itself with htmx 4.

Works well

  • Inline, no dialog. It stays in the drawer, doesn't cover the canvas, and doesn't depend on how the theme builds dialogs.
  • Focus handled on the server (FocusFirstCommand): into the row on open, back to Configure on close. It works with the keyboard since the toggles bind on click.
  • The data-uip-announce change event. Display Builder saves on change, so Cancel now works as a real revert: the restored values are saved back.

Could be better for a side panel

  • Configure under the widget, as a text button. It takes a full line per prop and comes after the value it applies to. A compact button on the label row, with aria-label="Configure source of …", would save that space. It's in the same place on every prop, right next to its label. Outside any summary, so no JS is needed.
  • Values hidden behind collapsed boxes. With 10 to 15 props, the drawer is a column of titles. Expanded props would show every value at a glance, see #3622982: Remove details element in component form?.
  • The announce event also fires after Save when nothing changed. Display Builder now skips such updates (#3623014: Fit UI Patterns source rows to the Config drawer's auto-save), but announcing only when the row's values changed, after Cancel or a source switch, would spare other hosts the request.

just_like_good_vibes opened merge request !565

pdureau’s picture

pdureau commented

Review of the current state of the MR 565. Commit: cd9d7dd5fcf36a0463ef3443ed7a891aef45a442

1. Replace details element by a simple "flow" of props ?

Moved to its own issue: #3622982: Remove details element in component form?

2. Open the AJAX inline instead of in modal?

UI/UX feedback

Icon in the details summary VS button in the inline modal ? It may be better to talk about that in an immediate follow-up issue. The priority here is to have something working.

Test with UI Patterns Field Formatters

✅ Still OK.

Test with UI Patterns Layout Builder

✅ Still OK.

Test with Display Builder

Tested with #3622023: Behaviors are not attached on htmx swaps outside the builder root (side panels).

✅ No need to explicitly click on "Update" button anymore. (⚠️ I got the reverse of the previous bug: some checkboxes widget, directly in the prop form, not in the inline modal, were not triggering update event, but it was seen only for a few seconds, in only a few props, and never again).

❌ I still get an a new page instead of a the inline AJAX sometimes. It is hard to identify the exact cause and give instructions to reproduce. but it never happens in PageLayout, it was only seen in EntityViewDisplay.

Test with UI Patterns UI

To check.

3. How to format PluginSettingsInterface::settingsSummary() ?

Not done in the MR yet, because we didn't define the expectations yet.

pdureau’s picture

pdureau commented

Tested with 1.0.x branch of Display Builder (188c2803f61e1df77b03d9596714fdb086fa807a)
Tested with EntityView buildable (/admin/structure/types/manage/article/display/default/display-builder)

Test 1: inline AJAX first

Same results after rebuilding the cache and not:

  1. I refresh the page
  2. I click on a Daisy UI Card to open the contextual form
  3. I click on URL prop to open the detail
  4. I click on "Configure"
  5. I pick "Token", i set my token ("[site:login-url]")
  6. ⚠️ I need to press Save twice, the first one submit, the second one close the inline modal
  7. ✅ OK, i see the URL in the Preview panel

Test 2: default widget first:

    Same results after rebuilding the cache and not:
  1. I refresh the page
  2. I click on a Daisy UI Card to open the contextual form
  3. I click on URL prop to open the detail
  4. I set /node value in the URL input widget
  5. I click on an other form widget to be sure I am switching focus
  6. ❌ the value is not updated in Preview Panel

Maybe it is because /node is not a "good" value for URL widget:

  1. I set http://example.org value in the URL input widget
  2. ✅ the value is updated in Preview Panel

So, we may need to print the AJAX form errors here.

Also, this error seems to break other widgets:

  1. I put back /node in URL input widget
  2. I click on "Action Tip" to open the details
  3. I switch the switch
  4. ❌ the value is not updated in Preview Panel
  5. I put back http://example.org in URL input widget
  6. I switch the switch in "Action Tip"
  7. ✅ the value is updated in Preview Panel

Test 3

Still investigating "I still get an a new page instead of a the inline AJAX sometimes."

pdureau’s picture

pdureau commented

Test again with 3 changes:

  • after updating Display Builder 1.0.x (now at 2d21deb82f787a5964ddf76111010fa3887a5bf2)
  • with an other theme: Bootstrap (so we know it is not theme specific)
  • with activating JS aggregation in /admin/config/development/performance

Same results so far.

Test 1: inline AJAX

  1. I refresh the page
  2. I click on a Bootstrap Button to open the contextual form
  3. I click on URL prop to open the detail
  4. I click on "Configure"
  5. I pick "Token", i set my token ("[site:login-url]")
  6. ⚠️ I need to press Save twice, the first one submit, the second one close the inline modal. It works well with keyboard input.
  7. ✅ OK, i see the URL in the Preview panel

Test 2: default widget

  1. I refresh the page
  2. I click on a Bootstrap Button to open the contextual form
  3. I click on URL prop to open the detail
  4. I set /node value in the URL input widget
  5. I click on an other form widget to be sure I am switching focus
  6. ❌ the value is not updated in Preview Panel ❌ and no error visible
  7. This seems to block the full form:
    1. I click on "Disabled" to open the details
    2. I switch the switch
    3. ❌ the value is not updated in Preview Panel
    4. I put back http://example.org in URL input widget
    5. ✅ the value is updated in Preview Panel
    6. I switch the switch in "Disabled"
    7. ✅ the value is updated in Preview Panel

Test 3

Still investigating "I still get an a new page instead of a the inline AJAX sometimes."

pdureau’s picture

pdureau commented

Test 1: inline AJAX

Fix found by Mikael:

+++ b/components/display_builder/js/display_builder.js
@@ -144,8 +144,12 @@
 
     builder.addEventListener('htmx:beforeRequest', (event) => {
       // Class used for opacity until request is finished.
-      // Avoid on preview.
-      if (!event.target.classList.contains('db-placeholder')) {
+      // Avoid on preview, and on the auto-save a form change sends: the
+      // lock would eat the click that follows the edit (mouseup lands on
+      // a page with pointer-events none, so no click is fired).
+      const autoSave =
+        event.detail.requestConfig.triggeringEvent?.type === 'change';
+      if (!event.target.classList.contains('db-placeholder') && !autoSave) {
         builder.classList.add('db-htmx-before-request');
       }

Test 2: default widget

Investigation in progress

Test 3

Investigation in progress

mogtofu33’s picture

mogtofu33 commented

Issue on Display Builder after investigation for partial possible fixes: #3623623: Contextual form loses clicks and invalid values silently

mogtofu33’s picture

mogtofu33 commented
Related issues: +#3623623: Contextual form loses clicks and invalid values silently
mogtofu33’s picture

mogtofu33 commented

It's the same fix as the one suggested in #3622928-13: UX of the asynchronous component form, with a possible fix for test 2 as well.

pdureau’s picture

pdureau commented

Tested with #3623623: Contextual form loses clicks and invalid values silently. It works

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: just_like_good_vibes » Unassigned
Status: Needs work » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.