- Issue created by @milos.kroulik
- 🇨🇿Czech Republic milos.kroulik
Perhaps https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21... could be used?
- 🇨🇿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.
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()'.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
2 months ago 9:24am 19 September 2024 - 🇩🇪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?