Note: include and to review/merge after #3628644: Scaffold components config array instead of string

Problem/Motivation

The components the Scaffold panel renders for real must be listed one id at a time. The Bootstrap recipe lists five, grid_row to grid_row_4.

UI Suite themes already group these components as group: Layout in their definitions: Bootstrap and DSFR with their grid rows, UIkit with its containers, grids and sections. Core copies group into the category the Component library panel groups by.

Proposed resolution

  • Add a Scaffold setting, a checkbox, off by default: render the components of the Layout group for real.
  • A component matches when its group is "Layout", case-insensitive.
  • Components matched by the toggle and the ids in components are both rendered for real.
  • The configuration summary says when the toggle is on.
  • The Bootstrap recipe turns the toggle on and drops its five ids.

Remaining tasks

User interface changes

A new checkbox in the Scaffold island settings of the profile form.

API changes

None.

Data model changes

display_builder.islands.scaffold gets a boolean, layout_group. No post update: a profile without the setting gets the default, off.

Issue fork display_builder-3628650

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

mogtofu33 created an issue. See original summary.

mogtofu33’s picture

mogtofu33 commented
Issue summary: View changes

mogtofu33 opened merge request !464

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Active » Needs review
pdureau’s picture

pdureau commented

#3628644: Scaffold components config array instead of string is RTBC

pdureau’s picture

pdureau commented

#3628644: Scaffold components config array instead of string is merged, so i rebase this one and review

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33

The initial idea is great (not having to manage component ID one by on, and working on "Groups" instead) but I am having doubt about hardcoding the "Group" name.

Groups are just documentation label with nothing special about them, and we have no control on them. For example, in DaisyUI, the "Layout" group is called "Grid".

Why not a combination of:

  • allowing wildcards in Render components. Example: ui_suite_bootstrap:grid_row_*
  • a select list instead of a checkbox to pick a specific group (or many?)

@jean, what do you think about this?

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau

Yes the group is a manual string, it will work only where theme properly use it, which are mostly ours. I had that in mind, that's why I didn't replace the list but made the toggle complementary.
Other option was to expose all groups, but was too much.

I am not sure about the wildcard, we don't do that anywhere as far as I remember, I prefer a specific list of ids so there is no surprise on what's expected and included.

pdureau’s picture

pdureau commented

Yes the group is a manual string, it will work only where theme properly use it, which are mostly ours.

Display Builder is not made for the few UI Suite themes we maintain but for all themes from contrib space

In sample of 33 contrib themes with SDC:

  • artisan
  • atlantic
  • bluecheese
  • bootstrap
  • bootstrap_italia
  • bulma
  • dropteam_ds_kit
  • drupal_cms_olivero
  • dsfr4drupal
  • etc
  • gcds
  • governor
  • kinetic
  • mercury
  • nttdata_theme
  • PatternFly
  • prototype
  • quartz
  • radix
  • space_ds
  • starlight
  • tailpine
  • ui_suite_bootstrap
  • ui_suite_daisyui
  • ui_suite_dsfr
  • ui_suite_material
  • ui_suite_nlds_rvo
  • ui_suite_protocol
  • ui_suite_uswds
  • umami
  • usb_sub
  • varbase_components
  • vartheme_bs5

We find only 2 themes using "Layout" group outside of ours:

$ grep -r "group: Layout" */components/**/*.component.yml | cut -d "/" -f 1 | uniq
mercury
tailpine
ui_suite_bootstrap
ui_suite_daisyui
ui_suite_nlds_rvo
ui_suite_protocol
ui_suite_uswds
Other option was to expose all groups, but was too much.

Unfortunately, a boolean property will be hard to evolve. I can keep the subject on my side until we find something.

mogtofu33’s picture

mogtofu33 commented
Status: Needs review » Postponed (maintainer needs more info)
Issue tags: -display_builder-1.0.0-beta9 +display_builder-1.0.0-beta

Set postpone until we have a better solution.

pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
Status: Postponed (maintainer needs more info) » Postponed
Issue tags: -display_builder-1.0.0-beta

Scaffold is not configurable anymore since #3630345: Preview as a switch on the Canvas, and in a new tab, instead of a split view 😱 A change done not mentioned/discussed, just after #3628644: Scaffold components config array instead of string, while we were discussing about the current feature.

What the hell has happened there? What about users currently using existing configuration on this island? And what does that means about this feature?

It is not only about configuration. With the removal of RealRenderTrait::buildComponentRealRender(), any proposal will also need to propose a way of embedding fragments rendered with front theme.

It is too much a mess for 1.0.0.