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

Fix issue #5349 by using a darker color as background when row selected #5364

Merged
merged 3 commits into from Sep 28, 2018
Merged

Fix issue #5349 by using a darker color as background when row selected #5364

merged 3 commits into from Sep 28, 2018

Conversation

@Songyu-Wang
Copy link
Contributor

@Songyu-Wang Songyu-Wang commented Sep 23, 2018

Fixes #5349

Screenshots
If applicable, add before and after screenshots to help show the issue being fixed, or the appearance of a new feature.

Before:
image

After:
image

@Songyu-Wang
Copy link
Contributor Author

@Songyu-Wang Songyu-Wang commented Sep 23, 2018

Please note light theme is also changed
image
Not sure if this is ok

Copy link
Member

@blink1073 blink1073 left a comment

LGTM, thanks!

@blink1073
Copy link
Member

@blink1073 blink1073 commented Sep 23, 2018

Copy link
Contributor

@tgeorgeux tgeorgeux left a comment

@Songyu-Wang do you mind trying "-jp-ui-inverse-font-color1", the Brand Color 1 leaves a bit to be desired on the light theme. The inverse color will make it more flexible going forward when more themes are incorporated!

Thanks again for working on this.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 28, 2018

do you mind trying "-jp-ui-inverse-font-color1", the Brand Color 1 leaves a bit to be desired on the light theme.

I tried it, and it doesn't work. For example, in the light theme the inverse font color is white, so you can't see the selection at all. In the dark theme the inverse font color is black, which is barely discernible against the dark grey background.

Perhaps themes need a new text selection color?

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 28, 2018

@tgeorgeux, @Songyu-Wang, how do you feel about extending @Songyu-Wang's original change to match the file browser selected colors at

.jp-DirListing-item.jp-mod-selected {
color: white;
background: var(--jp-brand-color1);
}

screen shot 2018-09-28 at 1 10 46 am

screen shot 2018-09-28 at 1 10 30 am

@tgeorgeux
Copy link
Contributor

@tgeorgeux tgeorgeux commented Sep 28, 2018

That works for me. I think if we make this change, we could also use the brand color/white text in the command palette on active per Issue #1095

@Songyu-Wang
Copy link
Contributor Author

@Songyu-Wang Songyu-Wang commented Sep 28, 2018

looks good to me

@tgeorgeux tgeorgeux merged commit 1a9c434 into jupyterlab:master Sep 28, 2018
1 of 2 checks passed
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 28, 2018

we could also use the brand color/white text in the command palette on active per Issue #1095

I think given that we want to use these sorts of things in the command palette, the filebrowser, and now the completer, that we should have a specific 'selected' background and font css variable color we can just apply wherever we want to show things selected in a list.

@blink1073 blink1073 mentioned this pull request Sep 28, 2018
31 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants