-
Notifications
You must be signed in to change notification settings - Fork 6
Fix: Layout conflict #264
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
base: 2.x
Are you sure you want to change the base?
Fix: Layout conflict #264
Conversation
Layouts from the localgov_paragraphs_layout submodule (e.g. Two column simple) are experiencing conflict with similar core layouts (e.g. Two column). This is because these layouts are using the same CSS selectors as the core ones (e.g. .layout--twocol). Renaming these CSS selectors resolves this.
Renamed selectors which should enable better style rule inheritance.
Retaining older CSS selectors for backward compatibility.
Preserving original CSS selector priority to retain backward compatibility.
|
@markconroy @finnlewis @Adnan-cds First, thank you for this PR Adnan. My learnings: /web/core/modules/layout_discovery/layout_discovery.layouts.yml is a good place to check the icon_map and see what the spec is for each core layout Core two columns specifies four regions (!): Two column bricks specifies seven: and so on. Any reason not to document the layouts we have this way? I seem to recall there was mention last week of some prior changes that these changes follow up? RecommendationsI think it's good to fix the conflict and distinguish the localgov layouts with their own classes but I don't think that layout--threecol-33-34-33--localgov naming is valid BEM We could (and probably should) go further on the consistent naming front and revise CSS file names (and any library refs to them) to match the changes. https://github.com/localgovdrupal/localgov_base/pull/833/files should be updated to match these changes Lots of refs in: Mitigating custom overrides / breaking changes: When updated and tested the two PRs should be released together with a minor (not patch) version bump since folks can be expected to follow localgov_base for breaking theme changes when updating their projects. |
|
Discussing in Merge Tuesday, @markconroy and @tonypaulbarker agree on the naming suggestion. Like layout--localgov-threecol-33-34-33 We should refactor to align on this. And align this too https://github.com/localgovdrupal/localgov_base/pull/833/files Also @tonypaulbarker mentions we should document this here in a README and perhaps in other places. |
|
@Adnan-cds is this OK? Harmonise on layout--localgov-threecol-33-34-33 style? |
|
Yeah, that's fine. I will try to complete the changes before the next Merge Tuesday. |
|
@Adnan-cds Any chance you could get these changes made so we could hopefully merge them tomorrow at Merge Tuesday? |
|
Sorry Mark, totally stuck on something else :( Just can't spare any time to look into LocalGov tasks for another month or so. But I have some downtime after this week. I will try to use that to get this ready for next week's Merge Tuesday if that's okay. |
|
Thanks Adnan, take care. |
|
Ready for review :) |
What does this change?
Layouts from the localgov_paragraphs_layout submodule (e.g. Two column (simple)) are experiencing conflict with similar core layouts (e.g. Two column). This is because these layouts are using the same CSS selectors as the core ones (e.g.
.layout--twocol). Renaming these CSS selectors resolves this.How to test
Configuration > Content authoring > Layout paragraphs settings > Layout sections(i.e./admin/config/content/layout_paragraphs/sections), enable these layouts: Two column, Three column 25/50/25, Three column 33/34/33, Two column (simple), and Three column 33/34/33 (simple). The last two of these layouts are provided by the localgov_paragraphs_layout submodule.Resolves #260