Problem/Motivation

While working on #3622928: UX of the asynchronous component form, we were also talking about keeping the details & summary HTML elements or not in component form.

Do we still need them now we have a simpler and more streamlined form?

Proposed resolution

The simplest change would be to do:

+++ b/src/Element/ComponentPropForm.php
@@ -133,7 +133,7 @@ class ComponentPropForm extends ComponentFormBase {
-        '#open' => FALSE,
+        '#open' => TRUE,

and:

.details.uip-prop-box > summary::marker,
.details.uip-prop-box > summary::before {
  display: none;
}

Which will generate something like that with Gin in Core:

We can also target something "cleaner" like:

But we may need to keep the prop form visually "boxed" when we are editing the complex sources in the (soon to be) inlined modal.

Let's talk.

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:

Comments

pdureau created an issue. See original summary.

mogtofu33’s picture

Some UX feedback based on this article which seems a good source NN/g: Accordions on Desktop: When and How to Use (Wang, 2023), as:

  • Avoid accordions "when there is minimal content": most props are one select.
  • "When your audience requires the majority or all the content… show all the content at once." Configuring a component means knowing all its values.
  • Use them "when the content is long and the window size is small": our grids.

Measured in the Display Builder drawer (936px tall, about 270px wide, 100-1500px height), with UI Suite Bootstrap components.

Header, 10 props Grid 1 column, 15 props
Boxes (today) 481px, 0 values visible 731px, 0 values visible
No boxes (#wrap FALSE) 1005px, 8 values visible 1509px, 15 values visible

Across the 46 components here, the median is 3 props: flat is short for most, long only for grids.

Proposal, in line with option 2:

  • Simple props flat: label and a small Configure button on one row, then the widget. The description and "Browse available tokens" on demand, not always shown. As it renders today, that clutter is about half the extra height.
  • Complex sources (component, menu, links) keep a box, with their value in the closed title, like "Toggler position: End". This depends on the summary format from #3622928: UX of the asynchronous component form.
  • Mark props set away from their default, so a 15-prop grid can be scanned.
just_like_good_vibes’s picture

Thanks @mogtofu33 for the reference and for providing some arguments to the balance.

We keep the current form for now as this issue was separated because it is not yet a priority.
It is important to remember that ui_patterns forms are used across many different parts of Drupal:
- Block configuration
- Field formatters
- Views styles, rows, and displays
- CKEditor 5
- Field Layout (core and soon contrib)
- Entity forms (via ui_patterns_field)
- Field widgets (upcoming)
- Layout Builder (core) and Display Builder (contrib)
- ...

Component complexity varies from site to site, ui_patterns provides a generic base, while ui_patterns_ui is here to address specific needs for site builders and editors (and it already allows to remove fieldsets, at the price of per-component configuration).

Because the available space varies significantly between all the mentioned contexts, any change in the main module should keep a universally compatible and convenient solution.

Some little comments about some points you raised :

- "Most props are one select": This depends entirely on the components available on a given site; And we are talking about slots and props in the general case. (in display_builder, only props are shown but this is an exception)
- "Configuring a component means knowing all its values": While some components are simple, others have many options (especially the ones coming from generic implementation of design systems). For a complex component, showing "everything" by default creates unnecessary noise for an editor who may only need to change a single value.
- "Median is 3 props": This data is based on UI Suite Bootstrap components within a specific builder (no slots shown) ? Our general-purpose forms should not be designed around one specific design system or one specific use case.

The question asked by the issue is not easy to answer and i personally believe we need to we need to address in fact more general architectural questions:
- what part(s) to keep in the main module, what part(s) could belong to ui_patterns_ui
- should we have a global rule for accordions, or should their behavior be dynamic based on the component's complexity?
- How should we handle the display logic for props and slots? For example, should we prioritize ordering based on SDC "required" attributes, or differentiate the display based on whether a value already exists or whether a complex selector is used?
- ...

let's find a solution that works for both simple and complex components across all Drupal contexts.

mogtofu33’s picture

Agreed that the default must stay safe for every context, and fine to keep the current form for now.

Your point about space varying a lot between contexts is actually why I think one global rule can't work.
A drawer, a modal and a full page form need different answers. Any single default will be a compromise for most of them.

On the data:

One theme is a small sample, so I measured five: Bootstrap, DaisyUI, DSFR, Material and USWDS, 206 components in total.

  • The median is 3 props overall and between 2 and 4 in every theme.
  • 74% of components have 5 props or fewer.
  • Bootstrap turns out to be the most complex of the five, so it was a demanding reference, not an easy one.
  • The large components are almost all grids: they are the largest three in four of the five themes.
  • Complex components do exist, but they are a minority, and a known one.

We can't speculate on every possible theme, but this is enough to pick a sound default.

The good news is that ui_patterns already has the mechanism: #wrap is set by the caller, and ui_patterns_field already uses it. It is just all or nothing today.

Proposal:

keep #wrap => TRUE as the default, so nothing changes for Block, Views, CKEditor 5 or Layout Builder (which by the way is deprecated...).
Let #wrap also accept a finer rule, for example a callback or a list of prop types or source plugins to wrap.
Then Display Builder can show simple props flat and keep boxes for complex sources, and other contexts keep what they have.

pdureau’s picture

Hello both,

This is getting exciting. We have many proposals popping up:

  • we keep the details elements:
    • just like it is today (@just_like_good_vibes)
    • but open by default
    • closed by default when no value set, open by default when a value set
    • but use them as "fieldset": not collapsible anymore (first screenshot of ticket description)
  • we remove the details element all together, and we get a flat list (@mogtofu33, and second screenshot of ticket description)

We can also imagine some hybrid (mixed of flat and details) or dynamic (flat or details according to some rules) proposals.

And some adjacent proposals:

  • display required props before the others (@just_like_good_vibes)

Let's talk about that with Christian and others, but the priority is still #3622928: UX of the asynchronous component form. This one can wait.

mogtofu33’s picture

Status: Active » Needs review

I pushed a concrete proposal for #4: besides TRUE/FALSE, #wrap now accepts a callable that decides per prop, from the source shown. The default does not change: Block, Views, CKEditor 5 and Layout Builder keep their details. About 20 lines, one kernel test.
Limits: props only, slots stay wrapped. If you prefer a declarative rule, for example a list of source tags not to wrap, I can switch to that.

Create a MR on display_builder to test it: #3624954: Config form usability

mogtofu33’s picture

I pushed two changes to 3623704-component-form-value-title. Both work without #3622982: Remove details element in component form?.

  1. A closed box names what is set in its title

The title becomes "Variant: [site:url]" instead of "Variant". The value comes from the first settingsSummary() line. Otherwise it falls back to the source label, except for an empty widget, which shows the plain title. The value sits in a so themes can style it, and it becomes part of the summary's accessible name.

This answers "which props already have values" without an icon, and without #open => TRUE. With #open, most boxes end up open after a few edits.

Test: ComponentPropFormTest::testBoxTitleShowsValue().

  1. The open button names its action

This follows #3: the two cases need different words. I went one step further and used the actual action.

Closed row shows Label Accessible name
A widget, editable in place Change source Change source: Variant
A summary hiding the value Edit Edit: Variant
Nothing set Choose source Choose source: Variant

"Edit" is also the default for slot rows. buildSelectorToggleButton() takes an optional label, so the close and cancel buttons are unchanged.

Test: ComponentPropFormTest::testOpenButtonNamesItsAction().

Guidelines:

Theme side effect: UI Suite Bootstrap adds icons by matching button text (Bootstrap::cssClassFromString(), "Configure" gets a gear). "Edit" now gets a pencil and "Change source" gets no icon. Choosing an icon from the label text is fragile, so maybe that's for a Bootstrap issue.

Merge note: this MR and #3622982: Remove details element in component form? both add a test at the same place in ComponentPropFormTest.php. Whichever lands second has a trivial conflict: keep both tests.

I set some screenshot of the resul in the display builder issue: #3624954: Config form usability

pdureau’s picture

Assigned: Unassigned » pdureau
mogtofu33’s picture

pdureau’s picture

Status: Needs review » Needs work
StatusFileSize
new22.79 KB
new24.46 KB

Review in UI Patterns

Test with ui_patterns_blocks's ComponentBlock in /admin/structure/block/manage/ui_suite_daisyui_button :

+++ b/src/Form/ComponentFormBuilderTrait.php
@@ -150,6 +150,7 @@ trait ComponentFormBuilderTrait {
       '#default_value' => $this->getComponentConfiguration($configuration_id),
       '#render_slots' => $render_slots,
       '#render_props' => $render_props,
+      '#wrap' => FALSE,
     ], $form_element_overrides);
     self::moduleHandler()->alter('ui_patterns_form', $form, $form_state);
     return $form;

Results:

  • the margin is too even and we don't know if the "Configure" button belong to the previous or the next prop
  • lack of markup allowing to add a visual delimitation of the inline modal (we don't know where it starts and ends)
  • the missing prop title (we don't know which prop we are editing).

I will check if I can propose a little something.

Review in Display Builder

Done in #3624954: Config form usability. Same feedback.