feat: introducing new hover focus options for editor.action.showHover

[idootop]

Background

The behavior of the editor.action.showHover action in vscode was changed in version 1.73.0 (#176057). Previously, it would show a hover, but now it only shows the hover if it is not already visible, and focuses on it if it is visible.

However, this change has caused issues for some third-party extensions, particularly translation plugins. These plugins rely on user text selection and use the editor.action.showHover action to display translation results in a hover.

Unfortunately, the new behavior introduced in vscode version 1.73.0 causes the editor to lose focus when subsequent showHover actions are triggered, preventing users from deleting or editing code and negatively impacting the user experience.

Related issues: #176640, intellism/vscode-comment-translate#181

The known affected extensions include:

Extension	Installs	Affected code
Comment Translate	265K	select.ts#L63
Translator Helper	8K	selection.ts#L52

What's this PR

This PR introduces a resolution for the following issues:

• fixes The editor loses focus when subsequent showHover actions are triggered #196890
• closes "Show or focus hover" possibly breaking behaviour #176640

A new enum has been integrated, offering three distinct hover focus states to enhance user control over the showHover action's behavior:

• noAutoFocus: The hover will not automatically take focus.
• focusIfVisible: The hover will take focus only if it is already visible.
• autoFocusImmediately: The hover will automatically take focus when it appears.

This enhancement provides developers with more flexibility and improves the user experience by allowing for customizable hover focus behavior.

[idootop]

@microsoft-github-policy-service agree

[idootop]

cc: @aiday-mar @alexdima Thanks ;)

[aiday-mar]

Hi @aeschli, what do you think about this proposal?

[aiday-mar]

After looking at the PR, I find the text descriptions confusing with the new option. Perhaps instead of introducing a new option we could add a new state to the existing focus option. Currently it is a boolean, but it could take one of the following three string values:

immediately -> when hover appears it will focus immediately
never -> the hover is never focused, even if already visible
ifVisible -> the hover is focused only if it already visible

In your case you would set the value of never on calling showHover.

What do you think @aeschli @alexdima ?

[idootop]

Hey @aiday-mar , I appreciate your detailed suggestion. It indeed presents a more comprehensive solution. I like the idea of extending the current focus option to have more states, which would offer us more flexibility and control. My only concern would be about backward compatibility since the original type of focus was a boolean.

Thanks for your rigorous and thoughtful input. Looking forward to discussing this further with you, @aeschli , and @alexdima .

[aiday-mar]

Hi @idootop, apologies for the delay in answering and thanks for the work so far. I discussed with the team and we think it would be a good change to indeed use the enum with the three possible states immediately, never and ifVisible. Feel free to implement this and we can merge the PR.

We will also do a hidden check within the code to verify if focus is a boolean in which case we will revert to one of the three states above. This should ensure backwards compatibility.

[idootop]

Hi @aiday-mar, thanks for the update. I agree, using the enum with the three states sounds like a good solution. I'll make sure to implement this later. Thanks again for your help!

[idootop]

Hi @aiday-mar,

I have completed the implementation of the enum with the three possible states (immediately, never, and ifVisible) as per our discussion. I have also thoroughly tested all the options to ensure everything works as expected. Could you please review the changes at your earliest convenience?

Thanks ;)

[aiday-mar]

Hey @idootop, I looked at the PR and pushed some code to polish the PR. More specifically I extracted an enum and removed the surfacing of the boolean option in the enum (but kept a check in the run method).

@aeschli Do you think you could give a second review to the PR? Thank you.