RFC Use Bootstrap components for styling#2452
RFC Use Bootstrap components for styling#2452tomhughes merged 5 commits intoopenstreetmap:masterfrom
Conversation
|
I think the issue was that autoprefixer-rails 9.x required sprockets 4 and we were still on 3 but we aren't any more so I can likely change that - will have a look later. |
This avoids clashing with including upstream bootstrap
This lets us work through things slowly as we use more features
gravitystorm
force-pushed
the
bootstrap
branch
from
34f9664 to
ec5039d
Compare
Thanks, I've rebased this branch now that is taken care of. |
|
OK, if nobody has any comments about either this PR or using an off-the-shelf style framework in general, then I think we should merge this and move on. The next steps would include removing our custom reset classes, reworking forms (see e.g. pictures at gravitystorm#37), working though the list of bootstrap components and enabling them all, and working though our stylesheets to see what can be removed and left to bootstrap instead. |
| * Copyright 2011-2019 The Bootstrap Authors | ||
| * Copyright 2011-2019 Twitter, Inc. | ||
| * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE) | ||
| */ |
There was a problem hiding this comment.
If this is upstream code shouldn't it be in vendor instead of app? I guess in truth it's a kind of halfway house in that this is a template that you're expected to edit?
Is this expected to go away once we are able to just enable the whole thing?
There was a problem hiding this comment.
Yes, it's a temporary thing. The guidelines suggest copy+pasting that file and working though it.
| @@ -1,4 +1,5 @@ | |||
| @import "parameters"; | |||
| @import "bootstrap-custom"; | |||
There was a problem hiding this comment.
How does this find the other file when the file starts with an underscore? Is there some rule where the scss preprocessor searches for files with the underscore?
What's the logic behind preferring a CSS level @import over a sprockets level require? Then again we already seem to have a mix of both and I'm not sure how the split was decided...
There was a problem hiding this comment.
I'm just following the documentation, I don't have any particular views either way.
| /* | ||
| *= require ltr/common | ||
| *= require bootstrap | ||
| *= require bootstrap-tooltips |
There was a problem hiding this comment.
Presumably a fairly short term goal will be to replace the existing vendored tooltip and dropdown support with something based on the bootstrap gem instead? I assume you've tested that there isn't any issue with mixing the two in the short term...
There was a problem hiding this comment.
Yes, I would expect to replace the vendored tooltip stuff and just use the upstream version. Currently the tooltips appear to work exactly as before (specifically black text on white background, bootstrap has vice-versa).
2 participants
I'm interested to hear what people think about this idea. I've discussed using bootstrap to guide our design in #2386 and also using the card components to spruce up the help page in #2321. I think a wider goal of replacing our custom (unmaintained) reset/style/grid system with bootstrap would be useful. I'm not suggesting here that we change our colours or layout, just that there's a lot of stuff that we can get (and easily upgrade in future) for free by using it.
I tried the approach of just throwing the whole of bootstrap into our stylesheet, which mostly worked but caused some glitches in rendering dropdowns and tooltips. So this approach instead has most of bootstrap disabled, and only certain components enabled, and keeps most of our styling intact. I started with the help page restyling to demonstrate how we can use one of the components.
The long-term goal with this would be to enable components individually, resolve any conflicts with the existing styles, and remove our custom framework/styling code for stuff that bootstrap can handle for us.
I'm not sure why autoprefixer-rails was constrained in 7c7ff4f so any comments on that are welcome too.