-
Notifications
You must be signed in to change notification settings - Fork 91
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
Sidebar fixes #1497
Sidebar fixes #1497
Conversation
I think the postcss autoprefixer plugin uses this information when proccessing our css file, and I'm trying to get it to avoid adding as many old rules.
This modernizes the components to be written in a way that we can more easily support rerendering pieces of the app. Flexbox also simplifies the sidebar calculations a lot, and should help a lot with mobile and localization (e.g. ltr-rtl) I'm also trying out a few new naming conventions that might help us work with our (admittedly very confusing and complex) UI code: - capital names for components (like React). `inspector` -> `Inspector` - dollar prefix for d3-selections (like JQuery) `sidebar` -> `$sidebar` - double dollar signs for _enter_ selections `sidebarEnter` -> `$$sidebar`
summary: - dragging sidebar resizer works - toggling in/out by clicking resizer works - persist sidebar preferences to local storage (closes #1479) - fix "viewport jumps after toggling sidebar" (closes #1468) (though the resizing is still jerky as it fights with map draws and I'd like to improve that.) - a few places rename systems to use shortnames (e.g. photosystem -> photos) - we don't need utilFastMouse anymore todo: - I left the sidebar resizer red - need to style it nicer - Remove that "Inspect" button? It's unnecessary with a proper resizer - ui.resize doesn't pan the map completely correctly to account for the resize I divided pan amount by half, expecting that the map will grow on both sides (from center) now But this still isn't totally right - the math needs to be based on main-map dims not sidebar dims. (for fun, this effect of weird pan is worse when the map is rotated)
Fixes are mostly - Don't add a `dir` attribute anywhere except at the top level (container) - Prefer `display: flex` where possible, as it respects that attribute. (aka avoid floated content, or special css rules to target `dir=rtl`)
Mentioned in 75ff758 Resizing (either the window or the sidebar) has been pretty janky and I'd like to improve this. This seems to happen for a few reasons - During a resize, the map dimensions, pixi dimensions, and temp transform of supersurface don't all agree. Trying to redraw when these are not agreeing causes jumpiness - I'm now classing the container as 'resizing' when the user is actively resizing something, we can use this information to avoid changing the pixi/canvas dimensions until the resize has settled. - Starting to track the difference between the canvas and map dimensions ("drift"?) and hopefully we can account for it by sliding the supersurface around. (to keep the map settled in one place until an actual redraw happens)
Now - `UiSystem.resize` will better handle the resizing to avoid having the map jump around during resizes. - `PixiRenderer` will avoid changing the transform between full draws and better manage the temporary transform set on the supersurface
Because resizing blanks the canvas, it's better to do it immediately before we instruct Pixi render to it. The old code did resize in APP, which meant that after resizing, the canvas would flash "black" until the next animation frame.
I, too, despise the inspect button. My thoughts:
-For mobile, we should probably include something for the user to click on to expand/contract, as edge-of-screen taps/gestures are really hard to do / already overloaded on a lot of phones/tablets. |
I would say yeah go with that- but pick whichever axis is longer to do the accordion thing (say, if the user has their phone on its side, we should absolutely change the flex flow and collapse left-right) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solid, proper, grade A.
height: 35px; | ||
.rapid-inspector { | ||
display: flex; | ||
flex: 1 1 0px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💪
(re: #1497) This also quietly changes a few of the text strings that we use to talk about this component. "sidebar" -> "feature inspector" Because if we move it to some other part of the screen like the bottom, it's not really a sidebar anymore
Cool, yeah I agree totally..
|
This fixes a few known issues with the sidebar
#1468
#1479
While in the sidebar code I did a bunch of other cleanups:
(this allows us to much easier keep track of how wide the user actually wants the sidebar to be)
OLD:
NEW: