search

atom / tabs

Tabs in Atom
MIT License
111 stars 115 forks source link

Perform left/middle click actions on `click` instead of on `mousedown` #515

Closed as-cii closed 6 years ago

as-cii commented 6 years ago

Fixes https://github.com/atom/atom/issues/15197 Fixes #198 Supersedes and closes #385

Other than simplifying the code (we don't need to schedule a setImmediate callback to focus the item corresponding to the clicked tab), this pull-request also serves as a workaround to a Chromium bug that blocks the UI thread when middle-clicking a tab on Linux and showing a message box in the handler.

Using click slightly changes the UX, because left and middle click actions will be performed when users actually end the click action, rather than when they start it. Chrome, Safari and the macOS UI all behave this way, so I believe this is a good tradeoff from a user-experience standpoint.

Please, note that we still need to handle the right-click event via a mousedown handler, because Chromium does not seem to fire clickevents when opening context menus.

@matthewwithanm: I saw you introduced the setImmediate scheduling for focus reasons. With these changes, pane activation seem to exhibit the correct behavior, so I went ahead and deleted that workaround. However, I'd ❤️ to get your feedback on this in case you think it's not quite we want.

@Ben3eeE @ungb @50Wliu: as mentioned above, this slightly changes the UX and rearranges the way we respond to mouse input events in tabs. Can you help me test this doesn't regress any unrelated feature? In particular, it may be good to test all the mouse interactions on the various platforms and ensure everything works as expected. I performed some tests on Ubuntu and macOS, but some more QA on this would be super appreciated. ✨

/cc: @nathansobo for 👀

matthewwithanm commented 6 years ago

@as-cii Yeah, the setImmediate() scheduling was already there, but only for the pane activation…I just moved the item activation into it and added the comment (#439).

All of that was specifically to work around the mousedown behavior, which I figured we had to keep since it was intentionally added in #124 but I don't really have any opinions about it. I also opened #440 which got rid of the need for the timeout but it kinda fell through the cracks.

Ben3eeE commented 6 years ago

LGTM ⚡️

I am unsure about changing the switch tabs on mousedown behavior in a hotfix release.

nathansobo commented 6 years ago

I think we should take the risk considering how terrible this is for Linux users.

robwierzbowski commented 6 years ago

Did this revert the performance improvements of https://github.com/atom/atom/issues/5174 / https://github.com/atom/tabs/pull/124? As a new Atom user coming from Sublime, waiting for mouseup (complete click) for tab switch feels very sluggish.

Agree, should click for some of the other actions I see in here (tab close?), but I feel for tab switching this is a regression.

I see the explanation that Chrome/Safari/MacOS all change on full click, but they're not the direct competitors of Atom, and I'm not comparing them directly when evaluating which editor to try next.