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

Fuzzy Select doesn't color currently selected item #195

Closed
Araxeus opened this issue May 9, 2022 · 6 comments · Fixed by #199
Closed

Fuzzy Select doesn't color currently selected item #195

Araxeus opened this issue May 9, 2022 · 6 comments · Fixed by #199

Comments

@Araxeus
Copy link

Araxeus commented May 9, 2022

I think match highlighting in #179 caused an undocumented feature regression/bug?

Fuzzy Select doesn't color the currently selected item

in v0.10.0 currently selected item was highlighted in blue (using ColorfulTheme)
but now in v0.10.1 it isn't anymore...

It's important to note that it doesn't work regardless of if .highlight_matches() is false or true

(since .highlight_matches(false) should behave exactly like in v0.10.0, I think this is a bug)


On a side note:

I set the highlight style to be Bold only using:

let theme = ColorfulTheme {
    fuzzy_match_highlight_style: console::Style::new().for_stderr().bold(),
    ..Default::default()
};

I think this looks better, so maybe bold should be the default instead of yellow+bold
(especially since the yellow color clash with the currently selected blue color that hopefully will be back)


@deg0nz
Copy link
Contributor

deg0nz commented May 9, 2022

You are absolutely right, this is a bug. I forgot to apply the style to the selected item.

I think this looks better, so maybe bold should be the default instead of yellow+bold

I intentionally chose yellow+bold because bold only was not enough contrast imho.

I tried different variants and I'm not really happy with any of them. Maybe the community can decide what they like best.
Here are they:

  1. Active item coloured and highlighted chars in active item bold:

fuzzy-select-highlight-bold

  1. Active item coloured and highlighted chars bold+yellow:

fuzzy-select-highlight-bold-color

  1. Active item coloured with highlighted chars bold only everywhere:

fuzzy-select-highlight-bold-all

I personally like no 1 best, tbh.
No2 is somewhat of colour-clashy, as @Araxeus correctly mentioned.
No3 has not enough contrast.

@Araxeus
Copy link
Author

Araxeus commented May 10, 2022

I personally like N3 best, I think the char highlight should be mellow and not super noticeable
(which is how it looks in vscode and how it was suggested in #163)

No1 is weird because the char highlight seems to disappear
(yes it becomes bold, but you don't expect it to be since all others are yellow)

No2 is color clashy indeed and shouldn't ever be a default

Anyways I think there should be some documentation about customizing this style, like the example I wrote above^
(best would be to make .highlight_matches() receive either a bool or console::Style but not sure how that would work)

@Araxeus
Copy link
Author

Araxeus commented May 13, 2022

@deg0nz could you open a PR that will fix the bug since you seem to have coded the fix already?

@deg0nz
Copy link
Contributor

deg0nz commented May 13, 2022

@Araxeus Yes, I just hadn't time to do so. Maybe I will get it managed later today. We'll see :)

@Araxeus
Copy link
Author

Araxeus commented May 13, 2022

If you think Bold isn't visible enough, console::Style::new().for_stderr().bold().italic() looks quite good imo

@deg0nz
Copy link
Contributor

deg0nz commented May 14, 2022

I decided to go for the bold only option. I think this is more of what initially was requested in #163.
The lack of contrast in my examples could come from the font use in my terminal and it's weight.

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 a pull request may close this issue.

2 participants