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

Autocomplete suggestion focus outline #1989

Merged

Conversation

liamcmitchell
Copy link
Contributor

Give autocomplete suggestion a visible focus outline to differentiate from hovered suggestion.

I got confused when searching with keyboard and my mouse was hovered over the suggestions.

Before:
image

After:
image

Give autocomplete suggestion a visible focus outline to differentiate from
hovered suggestion
@josevalim
Copy link
Member

Given many will use the keyboard, I wonder if we could have something more subtle? Perhaps a darker bg (on dark mode) for the mouse over one compared to the selected one?

@liamcmitchell
Copy link
Contributor Author

liamcmitchell commented Dec 30, 2024

Not sure what you mean by more subtle.

I think the hover style is fine on it's own. I wouldn't make it more subtle.

The focus indicator is similar to other elements:
image
I didn't notice that before and IMO they should be aligned - 2px instead of just 1px, visible keyboard focus is important.

Right now focus re-uses the hover style which is where the problem comes from. Focus and hover are independent states. An element can be both, either or none.

Maybe you want to differentiate more between hover and focus, so separate the hover style from focus?
image

@liamcmitchell
Copy link
Contributor Author

Another thought is to only show hover styles if a mouse has moved over the suggestions but I don't think it needs to be that complicated. Having two distinct styles for hover and focus is enough for me.

@josevalim
Copy link
Member

What if we go with GitHub's style and we only use the inset on the left? See the blue accent on the left in the screenshot below:

Screenshot 2024-12-30 at 12 55 55

@liamcmitchell
Copy link
Contributor Author

Works for me.

Screen.Recording.2024-12-30.at.13.55.43.mov

@josevalim
Copy link
Member

GORGEOUS.

@josevalim
Copy link
Member

@liamcmitchell just one check. We only need the inset now, no? We no longer need the different background color?

@liamcmitchell
Copy link
Contributor Author

I added a different background color for the selected state, matching GH. I think it's nice to have.

@josevalim josevalim merged commit 10c4e70 into elixir-lang:main Dec 30, 2024
5 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@liamcmitchell liamcmitchell deleted the autocomplete-suggestion-focus branch December 31, 2024 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants