➕ [#2176] added gesture handling for leaflet #519
Merged
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
bart-maykin
force-pushed
the
feature/2176-navigate-within-map
branch
from
1c12426 to
022d2f5
Compare
|
Jest tests seem to be failing because Leaflet (L) is not defined in the index.js file.
|
bart-maykin
force-pushed
the
feature/2176-navigate-within-map
branch
from
022d2f5 to
a25bfd2
Compare
sergei-maertens
requested changes
Aug 2, 2023
There was a problem hiding this comment.
Some more general remarks:
- Wrong default language - WCAG 3.1.2 failure elmarquis/Leaflet.GestureHandling#86 -> when switching the language in storybook, I'm only seeing English messages and never Dutch, that's wrong behaviour
- There's not a lot of activity in the library/repository, so this is a bit of a risk 🤔 if it works well, we can accept the risk
- two finger drag / page scroll prevention and glitchy panning elmarquis/Leaflet.GestureHandling#65 is concerning, have you seen similar behaviour?
bart-maykin
force-pushed
the
feature/2176-navigate-within-map
branch
2 times, most recently
from
5a5970e to
22eec73
Compare
|
bart-maykin
force-pushed
the
feature/2176-navigate-within-map
branch
from
22eec73 to
a2a2220
Compare
bart-maykin
force-pushed
the
feature/2176-navigate-within-map
branch
from
a2a2220 to
3add74e
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #519 +/- ##
==========================================
+ Coverage 46.56% 46.59% +0.03%
==========================================
Files 198 198
Lines 3518 3520 +2
Branches 632 632
==========================================
+ Hits 1638 1640 +2
Misses 1716 1716
Partials 164 164
☔ View full report in Codecov by Sentry. |
If we were to use the Map outside of React, then the gesture handling is available and easily enabled. This also refactors enabling the handling directly in the map container rather than relying on a useEffect call.
sergei-maertens
approved these changes
Aug 4, 2023
|
Cleaned up a little bit to register the handler globally and enable it via option so we don't need the component with side-effect. Tested on mobile with storybook and the panning/dragging is smooth for me too. |
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.
Fixes open-formulieren/open-forms#2176