Skip to content

add toggle details focus for terminal suggest widget#238022

Merged
meganrogge merged 26 commits intomainfrom
merogge/wip-details-focus
Jan 17, 2025
Merged

add toggle details focus for terminal suggest widget#238022
meganrogge merged 26 commits intomainfrom
merogge/wip-details-focus

Conversation

@meganrogge
Copy link
Copy Markdown
Collaborator

@meganrogge meganrogge commented Jan 16, 2025

want to support toggling focus between the suggest list and details for screen reader support.
fixes #238004

@meganrogge meganrogge requested a review from Tyriar January 16, 2025 04:05
@meganrogge meganrogge marked this pull request as draft January 16, 2025 04:05
@meganrogge meganrogge self-assigned this Jan 16, 2025
@meganrogge meganrogge added this to the January 2025 milestone Jan 16, 2025
@meganrogge
Copy link
Copy Markdown
Collaborator Author

meganrogge commented Jan 16, 2025

I figured parts 2 and 3 of the problem out! The key was the terminal wasn't receiving focus like it should've been when the list was focused.

Screen.Recording.2025-01-15.at.10.10.57.PM.mov

@meganrogge
Copy link
Copy Markdown
Collaborator Author

meganrogge commented Jan 16, 2025

Just need an idea now for a better way to handle the blur / determine when the widget should be closed. Tried checking the active element, but at the point I checked it, it wasn't yet the suggest details, it was this thing.

Screenshot 2025-01-15 at 10 28 25 PM

@meganrogge
Copy link
Copy Markdown
Collaborator Author

in the morning, i'll try to see how the editor handles this.

@meganrogge
Copy link
Copy Markdown
Collaborator Author

meganrogge commented Jan 16, 2025

Edit: figured that out too. Using addDisposableListener and focusout instead of instance.onDidBlur and checking event.relatedTarget.

@meganrogge meganrogge marked this pull request as ready for review January 16, 2025 05:22
@meganrogge meganrogge enabled auto-merge (squash) January 16, 2025 05:22
@meganrogge meganrogge marked this pull request as draft January 16, 2025 05:39
@meganrogge
Copy link
Copy Markdown
Collaborator Author

edit: nooo, it's not working. I had modified a when condition that made me think it was 🫠

@meganrogge
Copy link
Copy Markdown
Collaborator Author

the only way I can get this to work is via setting the precondition to be the opposite of the editor's. since these are the only commands with this keybinding, maybe that's fine 🤷🏼‍♀️

@meganrogge
Copy link
Copy Markdown
Collaborator Author

Screenshot 2025-01-15 at 11 57 08 PM

Tyriar
Tyriar previously requested changes Jan 16, 2025
@meganrogge
Copy link
Copy Markdown
Collaborator Author

fixed an additional problem where the widget wouldn't close on blur of the details and now is working really nicely

demo.mov

@meganrogge meganrogge marked this pull request as ready for review January 16, 2025 16:33
@meganrogge meganrogge requested a review from Tyriar January 16, 2025 16:33
@meganrogge meganrogge enabled auto-merge (squash) January 16, 2025 16:38
Tyriar
Tyriar previously approved these changes Jan 16, 2025
@meganrogge
Copy link
Copy Markdown
Collaborator Author

weird that this didn't get merged

@meganrogge
Copy link
Copy Markdown
Collaborator Author

Only change from yesterday is making sure it hides when the terminal is clicked - to match editor's behavior

@meganrogge meganrogge force-pushed the merogge/wip-details-focus branch from b4d120c to 4496961 Compare January 17, 2025 15:11
@meganrogge meganrogge merged commit 8159b3e into main Jan 17, 2025
@meganrogge meganrogge deleted the merogge/wip-details-focus branch January 17, 2025 15:52
@vs-code-engineering vs-code-engineering Bot locked and limited conversation to collaborators Mar 3, 2025
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.

add toggleSuggestionFocus command for simple suggest widget

3 participants