PB 1052: Add 3d tooltips for buildings #1226
Open
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
web-mapviewer
|
||||||||||||||||||||||||||||
| Project |
web-mapviewer
|
| Branch Review |
feat-PB-1052-add-3d-building-tooltips
|
| Run status |
|
| Run duration | 04m 20s |
| Commit |
|
| Committer | Martin Künzi |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
22
|
Skipped
|
0
|
Passing
|
233
|
| View all changes introduced in this branch ↗︎ | |
ltkum
force-pushed
the
feat-PB-1052-add-3d-building-tooltips
branch
from
5286671 to
62ea13a
Compare
pakb
reviewed
Jan 31, 2025
ltkum
force-pushed
the
feat-PB-1052-add-3d-building-tooltips
branch
2 times, most recently
from
944c71d to
f315c85
Compare
ltkum
force-pushed
the
feat-PB-1052-add-3d-building-tooltips
branch
3 times, most recently
from
1ccdcdb to
da706df
Compare
pakb
reviewed
Feb 11, 2025
There was a problem hiding this comment.
As said in the previous review's comment : we don't do layer specific layout/translation on the viewer side, that's something that has to be in the data, or it isn't.
Please remove all translation keys that aren't used directly by the viewer's code, and let the German text of buildings go through
packages/mapviewer/src/modules/map/components/cesium/CesiumInteractions.vue
Outdated
Show resolved
Hide resolved
packages/mapviewer/src/modules/map/components/cesium/CesiumInteractions.vue
Outdated
Show resolved
Hide resolved
packages/mapviewer/src/modules/map/components/cesium/CesiumInteractions.vue
Outdated
Show resolved
Hide resolved
packages/mapviewer/src/modules/map/components/cesium/CesiumInteractions.vue
Outdated
Show resolved
Hide resolved
packages/mapviewer/src/modules/map/components/cesium/CesiumInteractions.vue
Show resolved
Hide resolved
ltkum
force-pushed
the
feat-PB-1052-add-3d-building-tooltips
branch
from
14490c5 to
76ae789
Compare
pakb
reviewed
Feb 11, 2025
packages/mapviewer/src/modules/map/components/cesium/CesiumInteractions.vue
Show resolved
Hide resolved
packages/mapviewer/src/modules/map/components/cesium/CesiumInteractions.vue
Outdated
Show resolved
Hide resolved
ltkum
force-pushed
the
feat-PB-1052-add-3d-building-tooltips
branch
from
e9fff3b to
041543a
Compare
Issue : When we click on a 3d building, we want to see a tooltip containing some of that building information. Fix : we search the properties of the building to retrieve what we need.
- Added translation when there is no data in a field - Added handling when there is no data in a field - Added on the fly translation for the data in the fields - Removed some logging - we no longer recreate the same feature when someone clicks on the same building.
- Added names to the 3d layers - Added translation for the layer names - Added a check that allow the layers that are not part of the layer config to still have their name displayed
PB-1052: CSS adaptation Drawing infoboxes and building infoboxes now have an horizontal layout
- On hover, we now highlight the border of the buildings in blue - On click, they are now hovered in lime - upgraded to filter to filter out bridges and cableways - corrected some translations and translation keys usage
- Ensured building highlights were made with Cesium only - Replaced a deprecated method - Simplified the hovering interaction method in 3d
With the modularization of the mapviewer, a big rebase was in order. This commit seeks to make the small changes that were needed after the rebase to ensure everything works correctly Also ran the linter.
- We now base our colors for the highlighted buildings on colors from the mapviewer. - Removed refs where there were not needed - We now highlight all buildings under the mouse when clicking.
- We now give an object to the Geoadming3DLayer constructor and de-structure it, in the same manner as other layers in the code. - Removed the translations we do not want to do. ensuring we don't translate something
- We remove the building type from the tooltip as we don't want to display untranslated data to the user - Some linting - forcing new rendering upon moving the mouse on or off features, on when clicking
ltkum
force-pushed
the
feat-PB-1052-add-3d-building-tooltips
branch
from
041543a to
466d103
Compare
- Set the translations for 3d custom tooltips in their own locale json, separated from the rest of the translations - Added prefixes to know which layers these attributes correspond to, and avoid side effects - I wonder if anyone will read this - Parameters to create the tooltips are now in the cesium config, and the functions do not make special cases for individual layers. - Enforcing re-render when the highlights are supposed to change, removing the impression that the application is frozen.
ltkum
force-pushed
the
feat-PB-1052-add-3d-building-tooltips
branch
from
0538fc5 to
701b3f6
Compare
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.
Test link