-
Couldn't load subscription status.
- Fork 445
Add sticky marker tooltip for touch devices #1597
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
base: develop
Are you sure you want to change the base?
Add sticky marker tooltip for touch devices #1597
Conversation
…te a separate class function for 'updateHintPosition'
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.
Looks already pretty good
| } | ||
| }, | ||
| _setupTouchScreenTips() { | ||
| let markerHintTip = null; |
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.
Please add the variable to the context: this._markerHintTip
| this._map.on('pm:drawstart', (event) => { | ||
| if (event.shape === 'Marker' && !deviceHasFinePointer()) { | ||
| // Hides the "hint marker" as that is confusing for touch screen users. | ||
| const drawMarker = this._map.pm.Draw.Marker; |
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.
creating the variable drawMarker is obsolete
| permanent: true, | ||
| }).addTo(this._map); | ||
|
|
||
| const updateHandler = () => this._updateHintPosition(markerHintTip); |
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.
The call should be throttled this._markerHintTipUpdateHandler = L.Util.throttle(
Reference:
leaflet-geoman/src/js/Mixins/Modes/Mode.Drag.js
Lines 15 to 21 in d720305
| if (!this.throttledReInitDrag) { | |
| this.throttledReInitDrag = L.Util.throttle( | |
| this.reinitGlobalDragMode, | |
| 100, | |
| this | |
| ); | |
| } |
| markerHintTip = null; | ||
| this._map.off( | ||
| 'zoomlevelschange resize move', | ||
| this._updateHintPosition, |
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.
This doesn't remove the correct event listener function
| } | ||
| }); | ||
| }, | ||
| _updateHintPosition(markerHintTip) { |
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.
No need to have markerHintTip as argument, if assigned to the context this._markerHintTip
|
|
||
| // Creates the touch screen hint "permanently" (while using the tool) | ||
| // sticked at the top of the map. | ||
| markerHintTip = L.tooltip([0, 0], { |
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.
Not sure if we should use a plain html element, positioned absolute, instead. Then there would be no need to recalculate the position with every move
| _setupTouchScreenTips() { | ||
| let markerHintTip = null; | ||
|
|
||
| this._map.on('pm:drawstart', (event) => { |
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.
Eventlistener should be only added if the global option for mobile tooltip is enabled
This PR is created to add a feature requested in the issue #1595, which adds a sticky tooltip for the markers in case of the user not having a pointer device (e.g. a tablet or a phone).
It was confusing for touch screens to place the marker since it didn't have any feedback and sometimes the hint marker hides the actually added marker which can cause confusion.
More data and use case -examples in video form you can find by clicking the issue number.