JS library overrides are breaking layout builder interface

Created on 25 July 2024, 5 months ago
Updated 19 September 2024, 3 months ago

Problem/Motivation

radix_starterkit contains overrides of Drupal Core JS libraries (https://git.drupalcode.org/project/radix/-/blob/6.0.x/src/kits/radix_sta...). When I use them is my radix 6.x subtheme, they break layout builder interface in at least 2 ways:

  • when I try to add a block to a section, Bootstrap modal opens instead of the offcanvas panel. Its contents are are unstyled and it can't be closed
  • when I try to delete a block, bootstrap modal opens. After confirming the choice, removal does not happen and modal remains opened.

Proposed resolution

The overrides should not be applied in the layout builder interface.

Workaround

For now, I've commented out some of the overrides like this:

libraries-extend:
  #  core/drupal.ajax:
  #    - hd_radix/drupal.ajax
  core/drupal.checkbox:
    - hd_radix/drupal.checkbox
  #  core/drupal.dialog:
  #    - hd_radix/drupal.dialog
  #  core/drupal.dialog.ajax:
  #    - hd_radix/drupal.dialog.ajax
  core/drupal.message:
    - hd_radix/drupal.message
🐛 Bug report
Status

Needs work

Version

6.0

Component

Code

Created by

🇨🇿Czech Republic milos.kroulik

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @milos.kroulik
  • 🇨🇿Czech Republic milos.kroulik

    Or maybe https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21... (but it looks like it can be implemented only by the module.

  • First commit to issue fork.
  • Merge request !106move libraries-extend into .theme hook → (Open) created by windy-ux
  • Pipeline finished with Success
    5 months ago
    Total: 135s
    #237418
  • I tested both possible solutions hook_page_attachments_alter and hook_library_info_alter . However none of those hooks is able to change statically (in *.info.yml) attached libraries.
    They would have to be defined in 'libraries' within '*.info.yml' not as 'libraries-extend'.
    Because of that I move attaching those extending libraries into '*.theme' hook 'hook_page_attachments_alter()'.

  • Pipeline finished with Success
    5 months ago
    Total: 149s
    #239274
  • Right now the solution is not yet complete because all the libraries will be added to the page even though the original libraries (those being extended) are not included.
    hook_library_info_alter Can alter the libraries based on condition of original libraries presents(those being extended). However those gets cached and cannot be easily removed on pages containing layout builder.

  • 🇨🇿Czech Republic milos.kroulik

    It looks like this can be an inspiration for cache busting in hook_library_info_alter(). But this is something we can't do in the theme...

  • I don't think the issue is rather the way that we are overriding the core js files but rather the override itself, for now I did comment out the drupal.dialog and drupal.dialog.ajax overrides. with my latest changes to the dialog it works fine with modal and non modal dialog but still fails with the off_canvas.

    Until I manage to figure this out I believe disabling it would be the solution

  • 🇲🇾Malaysia ckng

    The current implementation of core library JavaScript overrides within the Radix sub-theme introduces several challenges:

    Low usage: Unless Bootstrap is used as admin theme or component such as modal is used in the frontend, these JS overrides are not needed at all.
    Tight Coupling: The sub-theme becomes overly coupled to specific JavaScript libraries and their implementations, limiting flexibility and maintainability.
    Subtheme Complexity: increased sub-theme complexity and making updates to the sub-theme potentially risky, hard to maintain.
    Dependency Issues: Changes in the core libraries could break the overrides, leading to unexpected behavior and requiring constant maintenance.

    To address these issues, I propose
    - Decoupling the core library overrides from the sub-theme, move them to the base theme and making them optional. - OR - Decoupling it to a module, making them optional, and can be enabled or disabled as needed.
    - Extend Libraries Rather Than Override: Prioritize extending core library functionality through inheritance or composition over direct overrides. Since what altered are mostly the markup, we only need to extend the behavior, prototype or function. Making it simpler to maintain and reduce conflict with changes from core.
    - Provide clear documentation on how to use the override module and how to extend core libraries effectively. This will help developers understand the available options and make informed decisions.

  • Thanks @ckng for the comment, to address some of it:

    - True that the usage of a core modal are low, but it would be ideal to make sure it still look the way it should in a Bootstrap env.
    - The same for off canvas but within layout builder ecosystem it's constantly being used.
    - Yes good idea to move it to the base theme
    - Core always breaks the themes! overriding it actually prevents the possible future breakage more compared to extending it. That aside I don't believe you can extend it.
    - Technically it's currently blocked since the override works for the off_canvas or for the modal, otherwise it breaks the other. for now it's commented out until we manage to fix it

  • 🇲🇾Malaysia ckng

    - Not every site utilizes the Layout Builder, too.
    - Looks like it is already using libraries-extend, and only override/redefine the specific behavior, or function.
    - Moving to base theme would ease theme maintenance greatly, thanks.

  • Status changed to Needs work 3 months ago
  • 🇩🇪Germany Anybody Porta Westfalica

    FYI: Just saw an implementation by @Grimreaper for Bootstrap modal in ui_suite_bootstrap theme: https://www.drupal.org/project/ui_suite_bootstrap/issues/3368697 Replace Core modal and offcanvas by Bootstrap's one Needs review

    Maybe it's helpful to have a look at the approaches there or even join forces? Another alternative might be to move the shared code into a module instead? Just some ideas so that not everyone cooks their own soup, as we say in Germany :D

  • 🇩🇪Germany Anybody Porta Westfalica

    PS:

    Moving to base theme would ease theme maintenance greatly, thanks.

    We just discussed the same. Currently being part of the kit means that fixes have to be recognized and then copied over each time, right?

    For the JS would make more sense to have the latest code in base theme, if I'm not missing something?

Production build 0.71.5 2024