Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Focus editor for tab after dragged over for 1500 millis #149604

Merged

Conversation

MachineMitch21
Copy link
Contributor

This PR implements #148664

This PR fulfills the requirements, but I think it could be better.
Perhaps we need a setting to indicate if this feature should be active
and also how long the delay should be before we focus the other editor?

@bpasero bpasero requested review from mjbvz and bpasero May 28, 2022 07:06
@bpasero
Copy link
Member

bpasero commented May 28, 2022

I pushed a small 💄 and think the code is generally OK to ship like this but I do agree:

  • do we need a setting?
  • what should be the default timeout to open the editor?

Some pointers:

  • macOS has an accessibility feature called "Spring loading delay" (System Preferences, Accessibility, Pointer Control) that enables/disables this for the macOS dock and allows to adjust the delay with a slider
  • our very own activity bar has a hardcoded delay of 800ms to open the view that is dragged over via DelayedDragHandler

My first thought was to align activity bar and tabs to be 800ms, but then I feel maybe users are getting frustrated that the active editor changes so quickly when you drag and drop over tabs?

@mjbvz thoughts on this?

Maybe also @daviddossett ?

@daviddossett
Copy link
Contributor

I think 800 is fine. Comparing the two..

800ms

800ms.mp4

1500ms

1500ms.mp4

It might make sense to consider this a setting that applies to these situations globally at some point so users can tune it. Kind of like the macOS example @bpasero mentioned. Rather than having a specific setting for each one, that is.

@bpasero bpasero marked this pull request as ready for review June 7, 2022 13:06
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just ship this in insiders and wait for feedback with 1500ms, then we can tweak according to real user feedback.

@bpasero bpasero merged commit e3a8e50 into microsoft:main Jun 7, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jul 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants