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

Do we still need bootstrap classes? #582

Open
keelanfh opened this issue Jul 3, 2023 · 5 comments
Open

Do we still need bootstrap classes? #582

keelanfh opened this issue Jul 3, 2023 · 5 comments
Labels
question Further information is requested

Comments

@keelanfh
Copy link
Member

keelanfh commented Jul 3, 2023

We still have quite a few bootstrap classes dotted about the distribution.

Just came up as an issue for me this morning as we had some analytics tags that were supposed to be tracking the start button but weren't, because someone had assumed that all instances of the start buttons had the same bootstrap classes, but some of ours didn't (depending on how they were set up).

At the very least, perhaps we could remove them from the default wysiwyg profiles - which would only affect new sites. I'd expect that most new sites are either subtheming localgov_base or creating their own theme which may or may not use Bootstrap - in any case, if they're going down that route, they'll probably make their own changes to these.

What do you think?

@keelanfh keelanfh added the question Further information is requested label Jul 3, 2023
@markconroy
Copy link
Member

They might be needed for legacy reasons in some of the modules, and were actively introduced in the localgov_eu_cookie_compliance module recently. We should look at removing them from modules if at all possible, and have them all written using BEM naming style, like we do in localgov_base.

There are none in localgov_base - no dependencies on external code was one of the principles in the rewrite of the localgov_base theme.

I'd like to see them removed from the wysiwyg profile too, but not sure exactly how we'd achieve that.

@keelanfh
Copy link
Member Author

It would be fairly easy to remove these from the wysiwyg profile for new installs, and that wouldn't have any impact on existing installs. I think that would be the safest option and I'd be in favour of that.

@keelanfh
Copy link
Member Author

I have an open PR for some changes to the wysiwyg config which I will update to include this.

@andybroomfield
Copy link
Contributor

Check to see if any are still present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants