8000 CSS/JS from localgov_base is not removed if it comes from an SDC component by JohnAlbin · Pull Request #281 · localgovdrupal/localgov_core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

CSS/JS from localgov_base is not removed if it comes from an SDC component #281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

JohnAlbin
Copy link
Contributor

Fixes #280

What does this change?

See #280 for the discussion.

How to test

  1. Add the localgov_base:add-to-calendar.twig component to an existing Drupal template. (For some reason I can't find any templates in LocalGov that are using the localgov_base components yet.)
  2. Notice that the add-to-calandar.css and add-to-calandar.js files are added to the webpage when the component is used.
  3. Turn on the "localgov_base_remove_css" and "localgov_base_remove_js" theme settings in your theme.
  4. Notice that the add-to-calandar.css and add-to-calandar.js files are STILL added to the webpage when the component is used.

How can we measure success?

  1. Apply the patch and retest with different combos of on/off for those theme settings. SDC creates a single library with both the CSS and the JS in it, but the new hook removes just the CSS or just the JS if that's what the theme settings are set to.

@JohnAlbin JohnAlbin changed the title CSS/JS from localgov_base is not removed if it comes from a component CSS/JS from localgov_base is not removed if it comes from an SDC component Apr 7, 2025
@JohnAlbin JohnAlbin force-pushed the 2.x-remove-components-css-js branch from aa592ee to b6ddf62 Compare April 7, 2025 17:02
@andybroomfield
Copy link
Contributor
andybroomfield commented Apr 7, 2025

Would localgov_bases .theme file be a more suitable place for this? Presumably this only effects themes that sub theme from localgov_base (which might be installable without localgov_core).

Noting the discussion on localgovdrupal/localgov_base#600 (comment) that we might want to move the components into localgov_core in which case I think the setting will also need to move.

@JohnAlbin
Copy link
Contributor Author
JohnAlbin commented Apr 8, 2025

Would localgov_bases .theme file be a more suitable place for this?

localgov_core.module is where localgov_core_template_preprocess_default_variables_alter() is currently located. And that hook sets the localgov_base_remove_css and localgov_base_remove_js twig variables that the localgov_base base theme and sub-themes use. So those theme settings already depend on localgov_core.

And while the hook_library_info_alter added in this PR can be put into a .theme file, hook_template_preprocess_default_variables_alter cannot go in a theme.

So it seemed to make more sense to put these two hooks next to each other in localgov_core.module.

Copy link
Member
@markconroy markconroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works a treat.

@finnlewis finnlewis merged commit 648ac05 into localgovdrupal:2.x May 6, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS/JS from localgov_base is not removed if it comes from an SDC component
4 participants
0