452/Fix network selector on mobile #503
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
|
| @@ -298,6 +299,10 @@ export default function NetworkSelector() { | |||
| const addPopup = useAddPopup() | |||
| const removePopup = useRemovePopup() | |||
|
|
|||
| // mod: When on mobile, disable on hover and enable on click events | |||
| const onHoverEvent = () => !isMobile && toggle() | |||
There was a problem hiding this comment.
while this works it doesn't really work if u resize the window, just kinda sucks. still approving tho
There was a problem hiding this comment.
But if you resize the window the user agent doesn't change.
If you want I could add that rule too, but doesn't work very well.
At first I tried addressing it by using the window size as reference
There was a problem hiding this comment.
I think the issue here is, the fact that we derive isMobile from the userAgent. As @W3stside points out, this doesn't work well with resizing windows. I think the better approach is to use media query detection and align the breakpoints (e.g. upToSmall etc.) with our existing media queries.
A library such as https://www.npmjs.com/package/react-responsive solves for that and comes with a onChange function.
There was a problem hiding this comment.
But window resizing doesn't change the fact that the cursor is still a mouse and will have a 'on hover' effect.
The point here is that on mobile there is no such thing, regardless of the screen size.
There was a problem hiding this comment.
@alfetopito Your point makes sense. I see we use a JS programmatic way of detecting the hover. E.g. with a CSS hover, on mobile the :hover is implied on click. So I get the current implementation.
That being said, I think there's value to go for mediaQuery detection vs. userAgent in general. Just in this case the only thing that I feel stands in the way here, is that we programmatically detect a HOVER event. And with mediaQueries you could technically detect a 'tablet' or 'medium' resolution which in actuality is a desktop without touch capabilities... So fine with with how it is 😅
Summary
Closes #452
Network selector is now clickable on mobile devices
To Test
2and3for another network