Page MenuHomePhabricator

Activate "jx-toggle-class" on click to fix broken mobile behavior
ClosedPublic

Authored by epriestley on Mar 28 2019, 11:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 13, 9:02 PM
Unknown Object (File)
Tue, Nov 26, 6:26 AM
Unknown Object (File)
Tue, Nov 26, 6:26 AM
Unknown Object (File)
Tue, Nov 26, 6:26 AM
Unknown Object (File)
Nov 20 2024, 1:57 PM
Unknown Object (File)
Nov 15 2024, 5:46 PM
Unknown Object (File)
Nov 12 2024, 10:26 AM
Unknown Object (File)
Nov 10 2024, 5:33 AM
Subscribers
None

Details

Summary

See downstream https://phabricator.wikimedia.org/T201480. Searching for things on mobile is a significant challenge because clicking the "Magnifying Glass" icon shows and then immediately hides the menu. I believe some aspect of iOS event handling has changed since this was originally written.

At some point, I'd like to rewrite this to work more cleanly and get rid of jx-toggle-class. In particular, it isn't smart enough to know that it should be modal with other menus, so you can get states like this by clicking multiple things:

Screen Shot 2019-03-28 at 4.51.07 PM.png (250×472 px, 46 KB)

This would also probably just look and work better if it was an inline element that showed up under the header instead of a floating dropdown element.

However, I'm having a hard time getting the Safari debugger to actually connect to the iOS simulator, so take a small step toward this bright future and fix the immediate problem for now: toggle on click instead of mousedown/touchstart.

This means the menu opens ~100ms later, but actually works. Big improvement!

I'd like to move away from "jx-toggle-class" anyway (it usually isn't sophisticated enough to fully describe a behavior) so reducing complexity here seems good. It isn't used in too many places so this is unlikely to have any negative effects, I hope.

Test Plan

On iOS simulator, clicked the magnifying glass icon in the main menu to get a search input. Before: got a search input for a microsecond. After: actually got a search input.

Diff Detail

Repository
rP Phabricator
Branch
touch1
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 22425
Build 30686: Run Core Tests
Build 30685: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Mar 29 2019, 12:00 AM
webroot/rsrc/js/core/behavior-toggle-class.js
19–20

(This comment is kind of pointless now, I'll just remove it.)

  • Remove the obsolete comment.
This revision was automatically updated to reflect the committed changes.