Skip to content

Conversation

mtsgrd
Copy link
Contributor

@mtsgrd mtsgrd commented Sep 24, 2025

Removes custom keyboard navigation for context menus, and replaces it
with the new focus manager functionality. Note that ContextMenuItem
contains two focusables, one for navigation and one to support f-keys.

Copy link

vercel bot commented Sep 24, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
gitbutler-web Ignored Ignored Preview Sep 24, 2025 2:22pm

@mtsgrd
Copy link
Contributor Author

mtsgrd commented Sep 24, 2025

output.mp4

@PavelLaptev could you have a look at this and let me know what you think? *edit: still need to fix tab for moving to next/prev item.

@PavelLaptev
Copy link
Contributor

@mtsgrd thanks for tagging. Looks nice, but let’s keep only one outline instead of two.

image

I think the inner one should stay since we use the arrow to navigate.

@mtsgrd
Copy link
Contributor Author

mtsgrd commented Sep 24, 2025

To me it looks like I included a change with outline: none to exactly prevent the double border. Could you be needing to recompile?

@PavelLaptev
Copy link
Contributor

To me it looks like I included a change with outline: none to exactly prevent the double border. Could you be needing to recompile?

@mtsgrd sorry, I was looking at the video you attached.

@PavelLaptev
Copy link
Contributor

@mtsgrd how can I trigger the context menu using the keyboard? I tried the "Enter" key but it didn’t open it.

Screen.Recording.2025-09-24.at.12.51.34.mov

@mtsgrd
Copy link
Contributor Author

mtsgrd commented Sep 24, 2025

I'll investigate!

@mtsgrd
Copy link
Contributor Author

mtsgrd commented Sep 24, 2025

Ok, should be fixed. I added enter key handling to KebabButton.svelte.

@PavelLaptev
Copy link
Contributor

Thanks! Looks like it works for branches but not for commits

Screen.Recording.2025-09-24.at.13.16.42.mov

Removes custom keyboard navigation for context menus, and replaces it 
with the new focus manager functionality. Note that ContextMenuItem 
contains two focusables, one for navigation and one to support f-keys.
@mtsgrd
Copy link
Contributor Author

mtsgrd commented Sep 25, 2025

@PavelLaptev just fyi, I still intend to land something like this but I'm currently trying to work through an inconsistency between Chrome and Safari wrt to :focus-visible. It's proving to be a bit tricky.

@PavelLaptev
Copy link
Contributor

@PavelLaptev just fyi, I still intend to land something like this but I'm currently trying to work through an inconsistency between Chrome and Safari wrt to :focus-visible. It's proving to be a bit tricky.

Gotcha! Let me know if you need me to test anything or if you need any help :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants