- Notifications
You must be signed in to change notification settings - Fork413
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix CSS overrides in applications#12646
Conversation
b717161 toc5c9ce1Compare@alecslupu I think there are actually two things here:
I think it'd be cleaner to work focus first in the bug fix (case 1), backport that and then work in the feature (case 2), do you agree with that approach? |
I would still consider it as a single fix. I would say that currently the The bad part when you load customized CSS within admin area, is that you may start break buttons, lists etc. To provide more context: We customized the frontend like this adding our overrides in We got the admin like this: We got the system panel like this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Tried it locally and it works as expected:
Before
After
Apart from the small changes that I'm suggesting, can you add some notes in the Releases Notes to communicate to implementers that this new way of overriding admin/system/design is available 🙏🏽 ?
decidim-core/app/packs/stylesheets/decidim/decidim_application.scss OutdatedShow resolvedHide resolved
decidim-system/app/packs/entrypoints/decidim_system_overrides.scss OutdatedShow resolvedHide resolved
Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
decidim-core/app/packs/stylesheets/decidim/decidim_application.scss OutdatedShow resolvedHide resolved
Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
Done in:b82f9a6 |
Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
Adding the |
* Declare new override entrypoint for frontend* Declare new override entrypoint for admin* Declare new override entrypoint for system panel* Add documentation* Add overrides in the design app to facilitate the development of new themes* Apply suggestions from code reviewCo-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>* Apply suggestions from code reviewCo-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>* Add docs* Add Readme* Lint* Apply suggestions from code reviewCo-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>---------Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
🎩 What? Why?
When we have switched away from webpacker to Shakapacker back into#10389, we accidentally introduced a bug regarding the CSS overrides. This may not be visible at the first hand, but when you try to create overrides for component css ( meetings, proposals), you are forced to use
!important.As per current implementation, the
decidim_application.scssis being imported as part ofdecidim_core.scss. However, in the Decidim + Shakapacker world, the component specific css is being required as a standalone file, which means that the css overrides is being loaded before the code that is supposed to override.This PR fixes this by adding 3 new entrypoints:
decidim_overrides,decidim_admin_overridesanddecidim_system_overrides, which are loaded after the other css has been loaded.Basically, in the frontend context:
Becomes:
This will allow any user to properly customize the front-end, admin or system areas of the platform.
📌 Related Issues
Link your PR to an issue
Testing
display: block