@gravitystorm approved this pull request.
I think this is great, and flipping between `flex-direction: column` and
`flex-direction: row` is about a million times easier to understand than all
the floating around that came before.
The main thing I was reviewing was whether the map pans correctly when sidebars
get open or closed, and I couldn't trigger any misbehaviour.
(OK, I admit I found one instance of misbehaviour. On small screen widths, if
you open the sidebar (e.g. Export) and then open the map-ui (e.g. Layers) and
then close and open the map-ui, the map slowly creeps northwards. But in
production, the map slowly creeps southwards, so that's not really a regression
in this PR).
I've suggested a couple of extras, but I'd be happy to merge this as-is.
> overflow-x: hidden;
overflow-y: auto;
These two overflows are set on all three sections (`#sidebar`, `#map`,
`#map-ui`) but `#map-ui` and `#map` both have their own overflow settings a few
lines further down.
It might be clearer to apply these two only to `#sidebar`.
> @@ -260,19 +257,6 @@ nav.primary, nav.secondary {
}
}
- .overlay-sidebar.overlay-right-sidebar {
`.overlay-right-sidebar` is now unused, I think? The javascript sets and unsets
the class, and there are (hard to understand) tests that check it, but it's not
otherwise used in javascript afaict and it's no longer in the css.
I'd be happy to see this removed, either here or in a followup PR.
> .overlay-sidebar #sidebar {
position: absolute;
- height: auto;
+ width: $sidebarWidth;
This is a little bit confusing, since at narrow screen widths the
overlay-sidebar is not actually 350 wide, that only really comes into effect at
md-up. But the code works as-is.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7207#pullrequestreview-4654985787
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/7207/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
