Chore: Refactor controls for mobile #174
Conversation
src/lib/Browser.js
Outdated
| @@ -264,6 +264,10 @@ class Browser { | |||
| return document.implementation.hasFeature('http://www.w3.org/TR/SVG11/feature#BasicStructure', '1.1'); | |||
| } | |||
|
|
|||
| static hasTouch() { | |||
| return 'ontouchstart' in window || (window.DocumentTouch && document instanceof DocumentTouch); | |||
There was a problem hiding this comment.
We could either use this (from modernizr), or always bind all events, or wait until a touch start as suggest by this article: https://medium.com/@david.gilbertson/the-only-way-to-detect-touch-with-javascript-7791a3346685
src/lib/Browser.js
Outdated
| * taken from Modernizr: https://github.com/Modernizr/Modernizr/blob/5eea7e2a213edc9e83a47b6414d0250468d83471/feature-detects/touchevents.js#L40 | ||
| * | ||
| * @public | ||
| * @return {boolean} is touch supported |
There was a problem hiding this comment.
Fixed, but just as a heads up I got a warning when I tried removing the return description.
src/lib/Controls.js
Outdated
| return !!element && element.classList.contains('bp-controls-btn'); | ||
| return ( | ||
| !!element && | ||
| (element.classList.contains('bp-controls-btn') || element.parentNode.classList.contains('bp-controls-btn')) |
src/lib/Controls.js
Outdated
| * | ||
| * @property {boolean} | ||
| */ | ||
| controlsFocused = false; |
There was a problem hiding this comment.
Since you are inside the Controls class already, this would be better named as isFocused
src/lib/Controls.js
Outdated
| event.preventDefault(); | ||
| // If we are not focused in on the page num input, allow hiding after timeout | ||
| if (!this.controlsFocused) { | ||
| this.blockHiding = false; |
There was a problem hiding this comment.
Don't need the if, both values are boolean, can just use assignment.
src/lib/Controls.js
Outdated
| * | ||
| * @property {boolean} | ||
| */ | ||
| blockHiding = false; |
There was a problem hiding this comment.
I recommend not using double negatives. Hard to read and easy to make mistakes. shouldNotHide = true might be better.
There was a problem hiding this comment.
Switching to shouldHide = true for now. This has no negatives by default and basically follows the opposite of isFocused. Makes sense to me but I'm open to other suggestions.
Tests to be added