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

Stronger wordHighlightBorder #40

Merged
merged 6 commits into from
May 20, 2020
Merged

Stronger wordHighlightBorder #40

merged 6 commits into from
May 20, 2020

Conversation

dcastil
Copy link
Contributor

@dcastil dcastil commented May 17, 2020

Closes #23, specifically addresses #23 (comment).

Kapture 2020-05-17 at 11 21 05

Kapture 2020-05-17 at 11 50 50

themes/dark.json Outdated
"editor.selectionHighlightBackground": "#17E5E600",
"editor.selectionHighlightBorder": "#17E5E644",
"editor.wordHighlightBackground": "#17E5E633",
"editor.wordHighlightStrongBackground": "#17E5E622",
Copy link
Contributor Author

@dcastil dcastil May 17, 2020

Choose a reason for hiding this comment

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

I intentionally put less focus on the strong word highlight than on the regular one for those reasons:

  • It's good to distinguish between the word where my cursor is and all the other occurrences to make it easier to navigate through the file.
  • However, we need to distinguish between a highlight and a selection, so the contrast between those needs to be sufficient as well. Making the strong highlight more transparent helps a lot here.
  • When we're searching for occurrences, we're not so much interested in the strong one.

Problems this could make:

  • Distracts from the place where we placed our cursor. I hope that won't be too much of an issue as the currently selected line gets its own highlight.

@dcastil dcastil mentioned this pull request May 17, 2020
Copy link
Contributor

@simurai simurai left a comment

Choose a reason for hiding this comment

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

I think this all makes sense, but I'm still somewhat on the fence. 😇

Distracts from the place where we placed our cursor. I hope that won't be too much of an issue as the currently selected line gets its own highlight.

Yeah, the idea of using only a border for wordHighlight is to avoid clashing with the selection. It should make sure that the selection is still easily visible without overlapping the wordHighlight.

Another reason is that wordHighlight is active by just moving the cursor around. In contrast the selectionHighlight is shown only when making a selection. So using a background for both makes that relationship more clear.

img-2020-05-19-16 59 03

I intentionally put less focus on the strong word highlight than on the regular one for those reasons

👍 Here another PR that reverts the border/background switch, but makes the "weak" border stronger: https://github.com/dcastil/github-vscode-theme/pull/1

@dcastil
Copy link
Contributor Author

dcastil commented May 19, 2020

That makes sense. Let's do it and I can report after a few days if something doesn't feel right. 😊

@dcastil dcastil changed the title Shift focus from selectionHighlight to wordHighlight Stronger wordHighlightBorder May 19, 2020
@simurai simurai merged commit fd3e8ee into primer:master May 20, 2020
@dcastil dcastil deleted the patch-1 branch May 20, 2020 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hard to see highlighted occurrences of marked variable in dark theme
2 participants