Skip to content
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

Remove duplicative grid package #286

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

ashleykolodziej
Copy link
Contributor

Fixes #285 by removing a duplicative reference to the responsive foundation grid package.

The purpose of the tools-cgb package in Responsive Foundation is to only print the exact CSS classes your plugin needs. This includes all icons and grid classes. The proper way to change this behavior is to change the variables above it:

$burf-extras:					false;
$use-default-icons:                        false;
$print-icon-classes:                       false;

Changing any of these values to "true" will print all classes out to the CSS, including all grid classes.

That said, that will re-enable this duplicative behavior. My strong suspicion is that we don't actually need all those classes. I don't see any references to the grid classes in the plugin except for in the Sass files, where placeholder classes are being used. I verified that this line was introduced here: ebd1a7d

This was probably to support extending the .col- classes. These have since been refactored to use placeholder classes, as intended, and it should now be safe to remove this import.

@ashleykolodziej ashleykolodziej self-assigned this Sep 18, 2020
@ashleykolodziej ashleykolodziej added 🔥 Product bug A bug which is not directly related to a custom child theme. ready for review labels Sep 18, 2020
# Conflicts:
#	dist/blocks.editor.build.css
#	dist/blocks.style.build.css
@ashleykolodziej
Copy link
Contributor Author

ashleykolodziej commented Apr 7, 2022

Visual regression between sandbox and ID demos: passed. The failure in this report is due to a long load time.
https://id-ashley.cms-devl.bu.edu/bu-blocks/
https://id-demos.cms-devl.bu.edu/bu-blocks/

Screen Shot 2022-04-08 at 10 18 20 AM

@ashleykolodziej
Copy link
Contributor Author

Visual regression on COM: passed
https://id-ashley.cms-devl.bu.edu/com/
https://wwwl.bu.edu/com/
Screen Shot 2022-04-07 at 9 38 00 PM

@ashleykolodziej
Copy link
Contributor Author

ashleykolodziej commented Apr 8, 2022

Visual regression on LAW: passed
https://id-ashley.cms-devl.bu.edu/law/
https://www.bu.edu/law/
Screen Shot 2022-04-08 at 9 58 01 AM

@ashleykolodziej
Copy link
Contributor Author

Visual regression on SPH: passed
https://id-ashley.cms-devl.bu.edu/sph/
https://www.bu.edu/sph/

Screen Shot 2022-04-08 at 10 33 27 AM

@ashleykolodziej
Copy link
Contributor Author

Copy link
Member

@acketon acketon left a comment

Choose a reason for hiding this comment

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

👍

@smtierneybu smtierneybu added the Requires Communication Plan Requires a plan to communicate with clients about potential impact label Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔥 Product bug A bug which is not directly related to a custom child theme. ready for review Requires Communication Plan Requires a plan to communicate with clients about potential impact ✅ VRT Completed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix repetitive CSS
3 participants