-
-
Notifications
You must be signed in to change notification settings - Fork 743
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
Fix NavigationControl compass regression #5205
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5205 +/- ##
==========================================
- Coverage 91.83% 91.82% -0.01%
==========================================
Files 281 281
Lines 38777 38786 +9
Branches 6756 6762 +6
==========================================
+ Hits 35609 35614 +5
- Misses 3041 3044 +3
- Partials 127 128 +1 ☔ View full report in Codecov by Sentry. |
Thanks!! Other places in the code are using: typeof ...
Let's keep it similar and avoid the need to write "window.". |
Rewrote it with |
Overall this looks great, thanks! |
Looks good! Can you add a changelog item? |
Done!
I can't run the existing test suite locally, so I wouldn't dare to try adding my own new stuff to it. |
Launch Checklist
See #5195 for discussion.
I think the regression was caused by "TouchEvent" only being defined on Chromium-derived and/or touchscreen-equipped browsers, such that the drag handler would fail on Firefox and Safari on my Mac laptop.
This PR does two things:
typeof TouchEvent !== 'undefined'
check to prevent a runtime error when it's undefined._onMouseEventOrTouchEvent
method to reduce duplication, possibly at the cost of clarity.CHANGELOG.md
under the## main
section.