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

Modernized DataTable styling #10363

Merged
merged 7 commits into from Aug 13, 2020
Merged

Conversation

philippjfr
Copy link
Contributor

@philippjfr philippjfr commented Aug 4, 2020

The default SlickGrid DataTable CSS styling looks like something straight out of the 90s. In this PR I've tried to update the styling to something more minimal and modern borrowing a little bit from the default styling of Pandas DataFrame reprs in notebooks:

Before:

Screen Shot 2020-08-04 at 1 08 38 PM

After:

Screen Shot 2020-08-04 at 12 39 35 PM

  • issues: fixes #xxxx
  • tests added / passed
  • release document entry (if new feature or API change)

@philippjfr
Copy link
Contributor Author

philippjfr commented Aug 4, 2020

Cc @bryevdv @mattpap

@mattpap
Copy link
Contributor

mattpap commented Aug 4, 2020

The header has too much contrast in my opinion. I would use a darker shade of gray instead of black.

@philippjfr
Copy link
Contributor Author

philippjfr commented Aug 4, 2020

Yeah, solid point, think I'd agree. I'll let @bryevdv chime in and then apply all the feedback at once.

@bryevdv
Copy link
Member

bryevdv commented Aug 4, 2020

I agree with the lighter grey for the header. I also think a very light grey dotted vertical column separator is ok. Maybe that would actually be worth making configurable through a property though.

@jbednar
Copy link
Contributor

jbednar commented Aug 4, 2020

By "header", do you mean the dividing line after the column headings? If so, then sure, a lighter line would be nice.

I'd want more contrast between the rows; the grey-row backgrounds seem too light to be visible (at least on my Retina laptop screen) once the borders are taken away.

@mattpap
Copy link
Contributor

mattpap commented Aug 4, 2020

By "header", do you mean the dividing line after the column headings?

Yes.

I'd want more contrast between the rows; (...)

A lot depends on the screen, because on my main I can see them fairly clearly, but on my laptop almost not at all. But in any case, I would increase that by at least a notch.

@bryevdv
Copy link
Member

bryevdv commented Aug 4, 2020

Also agree the bands could get slightly more contrast. If I were to make a short list of the highest value things that we could make configurable in future work it would be:

  • vertical sep
  • band color
  • index column color
  • selection row color
  • header color
  • fonts

@philippjfr
Copy link
Contributor Author

philippjfr commented Aug 13, 2020

What I settled on:

Screen Shot 2020-08-13 at 7 12 11 PM

@bryevdv
Copy link
Member

bryevdv commented Aug 13, 2020

LGTM @philippjfr I have updated the images and fixed the linter error. I am not sure whats up with the integration tests trying to load that header bg gif

@philippjfr
Copy link
Contributor Author

philippjfr commented Aug 13, 2020

Yeah, that's very odd, especially since it explicitly no longer references that image. Will try to dig into that now.

@mattpap
Copy link
Contributor

mattpap commented Aug 13, 2020

(...) especially since it explicitly no longer references that image.

It still does and requires background-image: none to work. This isn't limited to integration tests. I need to make sure that bokehjs tests report this as well in near future. I fixed the issue in the most recent commit.

@bryevdv
Copy link
Member

bryevdv commented Aug 13, 2020

Great I will merge this when green and then cut rc1 later this afternoon

@bryevdv
Copy link
Member

bryevdv commented Aug 13, 2020

Three similar failures for header-columns-over-bg.gif

@mattpap mattpap force-pushed the philippjfr/data_table_styling branch from 379f72f to fad720a Compare Aug 13, 2020
@mattpap
Copy link
Contributor

mattpap commented Aug 13, 2020

Fixed again. Hopefully it's the last one.

@mattpap mattpap merged commit 5c5e99f into branch-2.2 Aug 13, 2020
@mattpap mattpap deleted the philippjfr/data_table_styling branch Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants