PB 190 : alter tooltip position behaviour #685

ltkum · 2024-03-05T13:12:36Z

We are introducing a new url parameter : tooltipposition, with its store value in tooltipPosition.
It can take up four different values :

none : wont display tooltips. Clicking on a feature should remove this behavior
fixed : always display the tooltip in the infobox
floating : always display the tooltip on the map
default : the default value, which is the current behaviour (fixed on mobile, floating on desktop)

Test link

cypress · 2024-03-05T13:19:12Z

Passing run #1091 ↗︎

0	171	22	0	0

Details:

PB-190: all tests under one single it and some logs
Project: web-mapviewer	Commit: `f571745ceb`
Status: Passed	Duration: 03:58 💡
Started: Mar 12, 2024 4:44 PM	Ended: Mar 12, 2024 4:48 PM

Review all test suite changes for PR #685 ↗︎

ltshb

I think we should rethink the wording here, tooltip is by definition a small info box that is displayed on mouse hovering (https://en.wikipedia.org/wiki/Tooltip)

@pakb @boecklic @ltkum here is a proposal (feel free to comment improve it 😉 ):

URL parameter key := featureInfo
URL parameter values
- no parameter => no featureInfo => "none"
- no value (but parameter present) => "default"
- "DEFAULT" => base on UI size choose between "tooltip" and `"panel"
- `"panel"`` => actual infoBox, panel on the bottom of the map
- "tooltip" => actual floating tooltip

We could also use bPanel or bottomPanel instead of panel, so we could have other panel position in future.

This would also allow use to have window value for floating window (like in old viewer), or a fullscreen to have the info in fullscreen mode, but that's just thoughts for future.

Of course based on the acceptance and/or other proposal the code wordings needs to be adapted.

src/store/modules/ui.store.js

src/store/plugins/click-on-map-management.plugin.js

ltkum · 2024-03-07T07:46:57Z

@ltshb, @boecklic, @pakb : this seems like a good proposal to me. featureInfo seems like a good name.
For the future thoughts, I don't see any problems with the idea. I would prefer bottompanel rather than bpanel to make it easier to read and understand what it does, and I would also use this naming convention for the floating one, since this is technically not a tooltip by definition. I would go for :
featureInfo as a parameter, with the code using featureInfoPosition rather than tooltipPosition
and for the currently supported values :
bottompanel --> display the features informations in the infobox at the bottom
floatingpanel --> display the features informations on the map at the features coordinates
default --> depending on UI size, act as bottompanel or floatingpanel
none --> we do not show the feature info

boecklic · 2024-03-07T07:56:32Z

'featureInfo' makes sense to me too 👍🏻, maybe just one thought: we have kind of a "raw" featureInfo i.e. a json response from an API and a rendered featureInfo (aka htmlPopup). Currently we fully rely on the gtmlPopup, I could imagine that we'll simplify this in the future and just find a simple generic way to render directly the json in the popup...

ltshb · 2024-03-07T14:52:47Z

@ltshb, @boecklic, @pakb : this seems like a good proposal to me. featureInfo seems like a good name. For the future thoughts, I don't see any problems with the idea. I would prefer bottompanel rather than bpanel to make it easier to read and understand what it does, and I would also use this naming convention for the floating one, since this is technically not a tooltip by definition. I would go for : featureInfo as a parameter, with the code using featureInfoPosition rather than tooltipPosition and for the currently supported values : bottompanel --> display the features informations in the infobox at the bottom floatingpanel --> display the features informations on the map at the features coordinates default --> depending on UI size, act as bottompanel or floatingpanel none --> we do not show the feature info

@ltkum the current floatingTooltip is in my opinion a tooltip (although not floating), as it is attached to the feature with an arrow, the only difference to tooltip is that it is displayed on click event instead of on hover, but its look and feel is like a tooltip. Therefore I would keep tooltip instead of floatingpanel, it is also not floating as you cannot move it like in the old viewer where we have a floating window. For the the bottom panel please use camelcase bottomPanel

src/store/modules/ui.store.js

src/store/plugins/click-on-map-management.plugin.js

src/modules/infobox/InfoboxModule.vue

src/modules/map/components/cesium/CesiumMap.vue

src/modules/map/components/openlayers/OpenLayersHighlightedFeatures.vue

src/store/modules/position.store.js

ltshb

Do you plan to add e2e test in subsequent PR ?

src/modules/map/components/cesium/CesiumMap.vue

src/modules/map/components/openlayers/OpenLayersHighlightedFeatures.vue

src/router/storeSync/storeSync.config.js

src/store/modules/ui.store.js

ltkum · 2024-03-11T13:36:12Z

Do you plan to add e2e test in subsequent PR ?

I'll be working on that right now

ltshb · 2024-03-12T16:05:46Z

tests/cypress/tests-e2e/featurePreSelection.cy.js

+            it('Select a few features and shows the tooltip does not appear when featureInfoPosition is not specified', () => {
+                goToMapViewWithFeatureSelection()
+                checkFeatures()
+                checkFeatureInfoPosition(FeatureInfoPositions.NONE)
+            })
+            it('Shows the tooltip in its correct position when set to default (bottom Panel on Phone)', function () {
+                goToMapViewWithFeatureSelection(FeatureInfoPositions.DEFAULT)
+                checkFeatures()
+                checkFeatureInfoPosition(FeatureInfoPositions.DEFAULT)
+            })
+            it('Shows the tooltip on the map when featureInfo is set to tooltip, and handle strange cases', function () {
+                goToMapViewWithFeatureSelection('TOoLtIp')
+                checkFeatures()
+                checkFeatureInfoPosition(FeatureInfoPositions.TOOLTIP)


Do all the tests in one it(). it() are costly in term of cost in cypress cloud and performance

Added a showtooltip parameter which can be set to 'default', 'fixed', 'floating' or 'none'. Added the url parameter to track it. Modified the Infobox and OpenLayersHighlightedFeatures to use this parameter Removed the old 'floatingTooltip' parameter

…3d off

… its value was none PB-190: fixing tests lint PB-190: featureInfo position Renamed tooltip position parameter to featureInfo across the code Once we have decided on the parameter possibles values 'names', we'll change that

PB-190: renaming keys to pass the check in setFeatureInfoPosition PB-190: last nomenclature changes PB-190: nomenclature harmonization small nomenclature change small rebase artifacts accidentally made both the combo and the tooltip show use v-show instead of v-if in Infobox module for the infobox-content container

…d using a better computed getter in cesium map for tooltip position

PB-190 : small bugfixes dispatcher fix nomenclature harmonization

ltkum requested a review from ltshb March 5, 2024 15:12

ltkum force-pushed the PB-190-feat-tooltip-position branch 2 times, most recently from 74d7c4b to 9dbf2be Compare March 5, 2024 15:19

ltshb reviewed Mar 6, 2024

View reviewed changes

src/store/modules/ui.store.js Outdated Show resolved Hide resolved

src/store/plugins/click-on-map-management.plugin.js Outdated Show resolved Hide resolved

ltkum force-pushed the PB-190-feat-tooltip-position branch from 9dbf2be to 175e483 Compare March 7, 2024 07:47

ltkum force-pushed the PB-190-feat-tooltip-position branch 2 times, most recently from 0e9baac to acd4bc7 Compare March 7, 2024 08:55

ltkum requested a review from ltshb March 7, 2024 14:18

ltkum force-pushed the PB-190-feat-tooltip-position branch from 740805c to dd78c3b Compare March 7, 2024 16:42

ltshb requested changes Mar 11, 2024

View reviewed changes

ltkum force-pushed the PB-190-feat-tooltip-position branch 5 times, most recently from 40dc42a to 8c598a7 Compare March 11, 2024 12:34

ltkum requested a review from ltshb March 11, 2024 12:42

ltshb reviewed Mar 11, 2024

View reviewed changes

ltkum requested a review from ltshb March 12, 2024 10:06

ltkum force-pushed the PB-190-feat-tooltip-position branch 3 times, most recently from 73a559a to 2a92893 Compare March 12, 2024 12:16

ltshb approved these changes Mar 12, 2024

View reviewed changes

ltkum added 3 commits March 12, 2024 17:41

small safety around setCameraPosition to avoid errors when switching …

596e3f2

…3d off

PB-190: tooltip toggle available in 3d

c76db74

ltkum added 8 commits March 12, 2024 17:41

PB-190: ensuring url parameter is used correctly

7883f6d

PB-190: avoiding URL override when selecting features through URL, an…

d3fa5e7

…d using a better computed getter in cesium map for tooltip position

PB-190: featureInfo e2e tests

ac86e9c

PB-190: changing default behavior to 'NONE' rather than 'DEFAULT'

528fbcb

PB-190 : small bugfixes dispatcher fix nomenclature harmonization

PB-190: correct tests after change of default parameter for featureInfo

68eb228

PB-190: all tests under one single it and some logs

f571745

ltkum force-pushed the PB-190-feat-tooltip-position branch from 5adeb50 to f571745 Compare March 12, 2024 16:41

ltkum merged commit b9b01bb into develop Mar 12, 2024

ltkum deleted the PB-190-feat-tooltip-position branch March 12, 2024 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PB 190 : alter tooltip position behaviour #685

PB 190 : alter tooltip position behaviour #685

ltkum commented Mar 5, 2024 •

edited

Loading

cypress bot commented Mar 5, 2024 •

edited

Loading

ltshb left a comment

ltkum commented Mar 7, 2024

boecklic commented Mar 7, 2024

ltshb commented Mar 7, 2024

ltshb left a comment

ltkum commented Mar 11, 2024

ltshb Mar 12, 2024

PB 190 : alter tooltip position behaviour #685

PB 190 : alter tooltip position behaviour #685

Conversation

ltkum commented Mar 5, 2024 • edited Loading

cypress bot commented Mar 5, 2024 • edited Loading

Passing run #1091 ↗︎

Review all test suite changes for PR #685 ↗︎

ltshb left a comment

Choose a reason for hiding this comment

ltkum commented Mar 7, 2024

boecklic commented Mar 7, 2024

ltshb commented Mar 7, 2024

ltshb left a comment

Choose a reason for hiding this comment

ltkum commented Mar 11, 2024

ltshb Mar 12, 2024

Choose a reason for hiding this comment

ltkum commented Mar 5, 2024 •

edited

Loading

cypress bot commented Mar 5, 2024 •

edited

Loading