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

Command navigation highlight #145245

Merged
merged 4 commits into from
Mar 16, 2022
Merged

Command navigation highlight #145245

merged 4 commits into from
Mar 16, 2022

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Mar 16, 2022

Fixes #46561
Recording 2022-03-16 at 10 33 52

@Tyriar Tyriar added this to the March 2022 milestone Mar 16, 2022
@Tyriar Tyriar self-assigned this Mar 16, 2022
registerThemingParticipant((theme: IColorTheme, collector: ICssStyleCollector) => {
const focusBorderColor = theme.getColor(focusBorder);

if (focusBorderColor) {
Copy link
Contributor

Choose a reason for hiding this comment

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

as some people mentioned in the UX sync, perhaps this should intentionally be different from the focus border so people aren't expecting the focus behavior?

Copy link
Contributor

Choose a reason for hiding this comment

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

we could instead go with the gray that's used for the go to implementations decoration

Copy link
Member Author

Choose a reason for hiding this comment

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

This got me to realize it only showed focus border because of my theme:

      "editor.symbolHighlightBackground": "#0000",
      "editor.symbolHighlightBorder": "#399ef4",

Here's what it looks like on Dark+ without that:

image

Will try that and maybe add theme keys for it too

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, but an issue there is the background has to be drawn on top for us, so it would obscure the text...

Copy link
Contributor

Choose a reason for hiding this comment

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

that looks good to me. couldn't you set the transparency?

@Tyriar Tyriar merged commit 7d5f994 into main Mar 16, 2022
@Tyriar Tyriar deleted the tyriar/46561 branch March 16, 2022 20:49
@github-actions github-actions bot locked and limited conversation to collaborators Apr 30, 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.

Command tracking should show glyphs next to rows and flash lines when navigating
2 participants