PB-368 : fluid UI rework #773
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 failed tests on run #1949 ↗︎
Details:
|
|||||||||||||||
pakb
force-pushed
the
feat_PB-368_profile_tray
branch
from
d9a233f to
6950e6c
Compare
pakb
force-pushed
the
feat_PB-368_infobox_ui_rework
branch
2 times, most recently
from
bf39481 to
538b89b
Compare
pakb
force-pushed
the
feat_PB-368_infobox_ui_rework
branch
from
538b89b to
ff6c407
Compare
pakb
force-pushed
the
feat_PB-368_infobox_ui_rework
branch
4 times, most recently
from
75e143e to
f32fc18
Compare
1 task
remove the concept of FeatureCombo, as it is now entirely managed by the InfoboxModule component
pakb
force-pushed
the
feat_PB-368_infobox_ui_rework
branch
3 times, most recently
from
3d28eda to
f4d8f56
Compare
So that we do not need to have this overflow box (which is kind of hard to work with when placing things under) but can show the whole disclaimer as a tippy tooltip Also migrate HeaderWithSearchBar component to Composition API in the process
So that we are then ready to have a map viewport that doesn't take 100% of width and height, and identify will still work properly ClickInfo was also a class that was just there for data transfert, no method or similar things, so I changed it to a simple JSDoc definition and are now simple JSON objects
instead of the screen size, meaning we can resize the map container and identify will still work
in order to be able to use the new map size composable
searchbar, Swiss logo, etc...
no more position absolute made directly in the toolbar components, it will be placed correctly by the OpenLayersMap component
and let it be placed by its parent, instead of doing that itself
Profile is now displayed as a "tray" of the feature list, and the map isn't hidden by the infobox (or footer, or header) anymore. That means that if we do a zoomToExtent while we are looking at features in the infobox, the extent is perfectly placed in the visible viewport of the map
the selection of the correct "border" of the extent wasn't done properly, with a very wide screen the zoom would be too great. Instead choose to show the side of the extent that takes the most "space" on screen by taking the max calculated resolution.
remove vendor CSS and change what we need in :global blocs (without the position absolute the Cesium viewer was growing like crazy every frame, out of control...)
pakb
force-pushed
the
feat_PB-368_infobox_ui_rework
branch
from
f3767b6 to
1edf5f2
Compare
|
If you aren't into code review, please at least play with the test link and tell me if you find anything weird. 🙏 |
|
Diff has been saved as "snapshot" in this PR for further review |
Temporarily disabling 3D tests, as some rework of the Cesium component stack is required to make it work properly within the fluid UI (WIP that will be published in a separate PR)
pakb
force-pushed
the
feat_PB-368_infobox_ui_rework
branch
from
1edf5f2 to
180add3
Compare
ltshb
reviewed
Apr 29, 2024
Center (by default) the map prior to clicking on coordinate in util function.
pakb
force-pushed
the
feat_PB-368_infobox_ui_rework
branch
from
9b9b436 to
1b5ccc7
Compare
ismailsunni
reviewed
Apr 30, 2024
|
|
||
| import { useTippyTooltip } from '@/utils/useTippyTooltip' | ||
|
|
||
| const dispatcher = { dispatcher: 'ZoomOutButton.vue' } |
There was a problem hiding this comment.
Should it be ZoomInButton.vue?
ltshb
requested changes
Apr 30, 2024
There was a problem hiding this comment.
Unfortunately the user experience with having the map only on the available space is not a good idea. The main issue is that when menu/headers/footer/infobox is changing it changes the map viewport and the map is moved which give a bad user experience, having those components on top of the map provide a much better user experience in my opinion.
|
As discussed, this brings too many issue with map resizing for user experience to solve only one issue with the zoom to extent being sometimes out of the map. The idea of placing the menu and other things in a more fluid manner might be a good thing for a post-prod work, but we will focus on going live for the time being. (I'll keep the branch around in case I'd like to cherry pick things later) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Redo the UI from the ground up with the idea that a component shouldn't place itself in the "grid", but simply display what it has to show. Its parent is the one responsible to place it at the correct position.
The map now takes up the space available between the header and the footer (or between the header and the infobox, if the infobox is shown). Meaning it won't be "hidden" under both of these UI component anymore (even though it was fancy with some transparency).
This should greatly improve the experience of "Zoom to extent" on a selected feature, because now the feature will be center in the map viewport in any case.
Test link