Page MenuHomePhabricator

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

Authored by epriestley on Mar 28 2019, 11:54 PM.



See downstream 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:

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

rP Phabricator
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Mar 28 2019, 11:54 PM
epriestley requested review of this revision.Mar 28 2019, 11:56 PM
amckinley accepted this revision.Mar 29 2019, 12:00 AM
This revision is now accepted and ready to land.Mar 29 2019, 12:00 AM
epriestley added inline comments.Mar 29 2019, 12:21 AM

(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.