Unnecessary JS / CSS should not be loaded if search is disabled

The attached patch adapts @viappidu's work from https://www.drupal.org/project/olivero/issues/3136371 and only adds search related assets if the search module is enabled. The main difference is that it no longer relies on a theme setting.

@viappidu thanks for catching that I missed cleaning up these assets in my original patch!

Hi, provided patch removes search related assets, css and js if search module is disabled.

I am working on this issue and will try to get the same as mentioned in #4.

I have created a patch satisfying the asks in #4. Please review.

Thanks

Forgot to put it in needs review state. Apologies.

diff --git a/templates/layout/page.html.twig b/templates/layout/page.html.twig
index 90b1771..12f53c0 100644
--- a/templates/layout/page.html.twig
+++ b/templates/layout/page.html.twig
@@ -45,6 +45,15 @@
  */
 #}
 
+{# Attaching libraries for search blocks present at respective regions #}
+{% if page.primary_menu.olivero_search_form_narrow %}
+  {{ attach_library('olivero/search-narrow') }}
+{% endif %}
+
+{% if page.secondary_menu.olivero_search_form_wide %}
+  {{ attach_library('olivero/search-wide') }}
+{% endif %}

@himanshu_sindhwani you have to attach libraries from hooks, not using template

(Personally...) I agree with @kostyashupenko. Altering the block prerender I think would be the best choice.

@brianperry why wouldn't you use extra theme settings? Been busy lately but I started working on a patch to add missing extra (to my needs almost essential) functions like breacrumbs page titles a-la-bootstrap. (or, eg, button icons, extra js settings, ...)

Also, I'm not a great coder but wouldn't starting to code all functions as plugins (again like bootstrap) be beneficial?
I understand Olivero would a "base theme" so all functionalities might be intended as a proof of concept, but my first impression is that this theme is going to be great so making the subtheming the easiest possible, maybe even adding a "create subtheme" button, will have a great impact on "basic" end users. (Again my personal opinion, being one of those basic users).

This is a patch that adds search related libraries on preprocess block with region check.

Hi @vebrovski, thanks for the patch. I have already assigned the issue to myself and was just going to upload the fix for it. However, I reviewed your changes and found the changes in libraries.yml are missing in your patch.

I have achieved the asks in #8. Please review.

@himanshu_sindhwani sorry for the confusion. You are right, those libraries slipped away.

And thanks for your patch. I tested it and it applies libraries based on block status and region. I would just change elseif with if.

I will leave this as NR for somebody else to look at it.

Tested the latest patch and it works well. Search related assets aren't loaded during the minimal profile install, and with search enabled the assets are correctly loaded depending on which theme search related blocks are enabled. Since this patch doesn't really include my code anymore, thinking it is reasonable to set this to RTBC. Happy to have someone else weigh in as well if I'm too close to this one.

@viappidu the reason I removed the theme settings was due to @mherchel's feedback on https://www.drupal.org/project/olivero/issues/3136371 that he was hoping this could be solved without introducing a new theme setting. I don't know if his goal is to limit theme settings in general, but solving the issue at hand here can be done without new settings.

Also on the topic of Olivero as a base theme, I recall @mherchel saying that this theme wasn't intended to be used as a base theme. I can't remember the specific reasoning though - maybe he can comment on that.

@brianperry you guys are doing a great job so keep up with what you think is best.

Just to be more precise, when I said "base theme" what I really meant was "basic", meaning no extra settings and therefore not "base for other" themes. Though I keep thinking having some extras would attract a broader range of users without much effort if coded from the very beginning.

Will try to reproduce as @mherchel told and get a fix.

This should be good to go.
Thanks for picking up the issue in #18.

Hi, thanks for the patch. I don't know how to replicate this issue. I disabled/enabled the block and as before library loads. I deleted the block and recreated it, with same machine name, and the library loads.

But if you delete the search block from primary or secondary region and add a new one to it, the library will not load because of different machine name. And that is solved by this patch. It targets search_form_block (plugin id) inside specific region instead of olivero_search_form_narrow (block id).