Problem/Motivation

The "replace" replaces a existing component inside the find method. Layout builder and library using the plugin definition instead the replaced plugin. This leads to wrong slots/props in layout builder and library.

Issue fork ui_patterns-3523040

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

christian.wiedemann created an issue. See original summary.

christian.wiedemann opened merge request !375

christian.wiedemann’s picture

christian.wiedemann commented
Assigned: christian.wiedemann » pdureau
Category: Feature request » Bug report
Status: Active » Needs review
christian.wiedemann’s picture

christian.wiedemann commented

Can you check this if the behavior is right now. Now the component with "replaces" inside is taken into account for building slots and props in layout builder and library. But still both component appears. Is this right?

pdureau’s picture

pdureau commented
Assigned: pdureau » christian.wiedemann
Status: Needs review » Needs work

I think we need to leverage this Core issue #3391978: SDC: Add a method to retrieve only components not replaced by others to:

  • show all components in the component library (ui_patterns_library)
  • but only the non-replaced components:
    • in the components selector (ui_patterns) when #render_slots is true
    • in specific integration when #render_slots is false (so in ui_patterns_layouts)

If we don't want to wait for the Core issue to be fixed, let's accelerate it and make a compatible change while waiting.

Don't you think?

christian.wiedemann’s picture

christian.wiedemann commented

Yes you are right but I think we should also hide the component in the library.

christian.wiedemann’s picture

christian.wiedemann commented

So I hide the components with a "replaces" from component selector / Layout and UIP UI. Inside the library it is a bit more complicated. Right now both components are visible but htis does not make sense from my point of view.

We should also hide the "replaces" component and inside the single view of the component we should switch from "replaces" to "Replaced by". Good?

pdureau’s picture

pdureau commented
Issue summary: View changes

Yes you are right but I think we should also hide the component in the library.

I don't have strong opinion about this. So, let's do as you want or let's ask the team.

We should also hide the "replaces" component and inside the single view of the component we should switch from "replaces" to "Replaced by". Good?

We must hide the replaced component and show only the replacing component, right? So what would be the use of showing "Replaced by" info in replaced components?

pdureau’s picture

pdureau commented
Title: [2.0.x] Ensure using always the "replaced " component structure » [2.0.4] Ensure using always the "replaced " component structure
christian.wiedemann’s picture

christian.wiedemann commented

For clearification:
A Card from a base theme (BaseCard) is overwriten with a SubCard inside a theme.
So right now I hide SubCard everywhere because this SubCard should not be used for mapping or something else. It should always used the BaseCard as mapping target but with props/slots and template from the SubCard. We must ensure that already existing configs are working so we can't hide the BaseCard. Inside the library I would also recommend to hide the SubCard and only render the BaseCard with new slots and props and twig template and only say with "Replaced By" that the subcard is used here.
Hope it is more clear now.

pdureau’s picture

pdureau commented

So, your proposal is:

  • On the Component selector and the Library, show only the replaced component ("BaseCard") with its ID
  • But, once selected, show the form of the replacement ("SubCard")

What happen if a component is replaced by many others ("SubCard1" and "SubCard2") ? What happen if a replacement is replaced ("SubSubCard") ? I guess we need to follow the SDC rendering logic.

pdureau’s picture

pdureau commented

Also, what about stories? We merge?

pdureau’s picture

pdureau commented
Title: [2.0.4] Ensure using always the "replaced " component structure » [2.0.5] Ensure using always the "replaced " component structure
christian.wiedemann’s picture

christian.wiedemann commented
Status: Needs work » Needs review
christian.wiedemann’s picture

christian.wiedemann commented

I finalized the library integration and fixed some implementation detaisl. Ready to merge from my side.

I also checked the negiation mechanisms for the component itself. It is quite straightforward. It is done in ComponentNegotiator with. First it is checked if the component is in your theme and than the module weight is used for filtering the right component id. 

$matches = array_filter(
      $all_definitions,
      static fn(array $definition) => $component_id === ($definition['replaces'] ?? NULL),
    );
    $negotiated_plugin_id = $this->maybeNegotiateByTheme($matches);
    if ($negotiated_plugin_id) {
      return $negotiated_plugin_id;
    }
    return $this->maybeNegotiateByModule($matches);

So there is only one replacement which is taken. The patch uses also the ComponentNegotiator to return the right id for the definition itself.

pdureau’s picture

pdureau commented
Assigned: christian.wiedemann » just_like_good_vibes

To review.

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: just_like_good_vibes » christian.wiedemann
Status: Needs review » Needs work

As we discussed,
it would be better to have two new methods in the decorated sdc service, to be able to get SortedDefinitions and getGroupedDefinitions with replacements.

we need nice names :)

christian.wiedemann’s picture

christian.wiedemann commented

I added the two new functions getNegotiatedSortedDefinitions and getNegotiatedSortedDefinitions getNegotiatedGroupedDefinitions and adjust all the places where we need the "real" definition".

We can discuss this with Pierre if we may move this to core.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented

as discussed, we will merge stories form replaced component and replacer

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: christian.wiedemann » Unassigned
Status: Needs work » Fixed
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Fixed » Needs work

Hello,

I am updating on 2.0.5 on Sobki and I got the following error:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "ui_patterns:sobki_theme_bootstrap:grid_row_2" plugin does not exist. Valid plugin IDs for Drupal\Core\Layout\LayoutPluginManager are: layout_twocol_section, layout_threecol_section, layout_fourcol_section, layout_onecol, layout_twocol, layout_twocol_bricks, layout_threecol_25_50_25, layout_threecol_33_34_33, navigation_layout, layout_builder_blank, ui_patterns:ui_suite_bootstrap:accordion_item, ui_patterns:ui_suite_bootstrap:accordion, ui_patterns:ui_suite_bootstrap:button, ui_patterns:ui_suite_bootstrap:button_group, ui_patterns:ui_suite_bootstrap:button_toolbar, ui_patterns:ui_suite_bootstrap:close_button, ui_patterns:ui_suite_bootstrap:card_body, ui_patterns:ui_suite_bootstrap:card, ui_patterns:ui_suite_bootstrap:card_group, ui_patterns:ui_suite_bootstrap:card_overlay, ui_patterns:ui_suite_bootstrap:carousel_item, ui_patterns:ui_suite_bootstrap:carousel, ui_patterns:ui_suite_bootstrap:modal, ui_patterns:ui_suite_bootstrap:offcanvas, ui_patterns:ui_suite_bootstrap:dropdown, ui_patterns:sobki_theme_bootstrap:grid_row_2_33_67, ui_patterns:sobki_theme_bootstrap:grid_row_2_25_75, ui_patterns:sobki_theme_bootstrap:grid_row_2_67_33, ui_patterns:sobki_theme_bootstrap:grid_row_2_75_25, ui_patterns:sobki_theme_bootstrap:grid_row_3_50_25_25, ui_patterns:sobki_theme_bootstrap:grid_row_3_25_50_25, ui_patterns:sobki_theme_bootstrap:grid_row_3_25_25_50, ui_patterns:ui_suite_bootstrap:grid_row, ui_patterns:ui_suite_bootstrap:grid_row_1, ui_patterns:ui_suite_bootstrap:grid_row_2, ui_patterns:ui_suite_bootstrap:grid_row_3, ui_patterns:ui_suite_bootstrap:grid_row_4, ui_patterns:ui_suite_bootstrap:list_group_item, ui_patterns:ui_suite_bootstrap:list_group, ui_patterns:ui_suite_bootstrap:navbar_nav, ui_patterns:ui_suite_bootstrap:navbar, ui_patterns:ui_suite_bootstrap:breadcrumb, ui_patterns:ui_suite_bootstrap:pagination, ui_patterns:ui_suite_bootstrap:nav, ui_patterns:ui_suite_bootstrap:alert, ui_patterns:ui_suite_bootstrap:badge, ui_patterns:navigation:badge, ui_patterns:ui_suite_bootstrap:figure, ui_patterns:ui_suite_bootstrap:spinner, ui_patterns:navigation:title, ui_patterns:navigation:toolbar_button, ui_patterns:ui_suite_bootstrap:progress, ui_patterns:ui_suite_bootstrap:progress_stacked, ui_patterns:sobki_theme_bootstrap:animated_number, ui_patterns:sobki_theme_bootstrap:container, ui_patterns:sobki_theme_bootstrap:dropdown_megamenu, ui_patterns:ui_suite_bootstrap:table_cell, ui_patterns:ui_suite_bootstrap:table_row, ui_patterns:ui_suite_bootstrap:table, ui_patterns:ui_suite_bootstrap:toast, ui_patterns:ui_suite_bootstrap:toast_container, ui_patterns:ui_suite_bootstrap:blockquote, ui_patterns:ui_suite_bootstrap:list in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 53 of core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php).

It seems that now components with a "replaces" key does not appear anymore in the available plugins.

Also I saw in the commit a change in the library template, should a change record be made?

christian.wiedemann’s picture

christian.wiedemann commented

Hi,
this is done by design because site builders should always use the base component in configuration. So ui_patterns:ui_suite_bootstrap:grid_row_2 instead of ui_patterns:sobki_theme_bootstrap:grid_row_2.
The patch fixes the overwrite problems. So you see all slots/props from the replaced component inside layout builder.

The problem is that existing configs could be a problem. Especially content overwrites. Maybe we readd "replaces" components for layouts.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Thanks for the reply.

Then in this case what is missing is an update hook for existing config and layout overrides.

Without this update, not possible to fix those config (except by editing config files or from admin) and not possible to fix layout overrides due to fatal error.

I can share some code for looping on layout overrides if needed.

pdureau’s picture

pdureau commented
Title: [2.0.5] Ensure using always the "replaced " component structure » [2.0.6] Ensure using always the "replaced " component structure
Assigned: Unassigned » christian.wiedemann
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Title: [2.0.6] Ensure using always the "replaced " component structure » [2.0.5] Ensure using always the "replaced " component structure
Assigned: christian.wiedemann » Unassigned
Status: Needs work » Fixed
christian.wiedemann’s picture

christian.wiedemann commented

Status: Fixed » Closed (fixed)

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