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

[Table] Add sticky header support #17139

Merged
merged 22 commits into from Aug 28, 2019
Merged

[Table] Add sticky header support #17139

merged 22 commits into from Aug 28, 2019

Conversation

egerardus
Copy link
Contributor

@egerardus egerardus commented Aug 24, 2019

Closes #6625

Added a scrollable table demo (scrollable rows with a fixed header and pagination toolbar) because it seems like a really common use case.

Capture d’écran 2019-08-27 à 20 09 08

egerardus and others added 4 commits August 24, 2019 13:19
Added link to ScrollableTable.js example
Then example code for scrollable table rows with a fixed header and toolbar.
@mui-pr-bot
Copy link

mui-pr-bot commented Aug 24, 2019

Details of bundle changes.

Comparing: 8ec0e6f...f95512e

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.05% 🔺 +0.08% 🔺 329,524 329,697 90,000 90,075
@material-ui/core/Paper 0.00% 0.00% 68,774 68,774 20,487 20,487
@material-ui/core/Paper.esm 0.00% 0.00% 62,148 62,148 19,215 19,215
@material-ui/core/Popper 0.00% 0.00% 28,468 28,468 10,177 10,177
@material-ui/core/Textarea 0.00% 0.00% 5,094 5,094 2,137 2,137
@material-ui/core/TrapFocus 0.00% 0.00% 3,834 3,834 1,614 1,614
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,386 16,386 5,827 5,827
@material-ui/core/useMediaQuery 0.00% 0.00% 2,541 2,541 1,058 1,058
@material-ui/lab 0.00% 0.00% 153,304 153,304 46,710 46,710
@material-ui/styles 0.00% 0.00% 51,492 51,492 15,305 15,305
@material-ui/system 0.00% 0.00% 15,658 15,658 4,371 4,371
Button 0.00% 0.00% 78,778 78,778 24,073 24,073
Modal 0.00% 0.00% 14,346 14,346 5,009 5,009
Portal 0.00% 0.00% 2,907 2,907 1,319 1,319
Rating 0.00% 0.00% 70,245 70,245 21,942 21,942
Slider 0.00% 0.00% 74,483 74,483 23,065 23,065
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 52,305 52,305 13,790 13,790
docs.main 0.00% 0.00% 597,298 597,298 190,849 190,849
packages/material-ui/build/umd/material-ui.production.min.js +0.06% 🔺 +0.08% 🔺 300,429 300,602 86,412 86,483

Generated by 🚫 dangerJS against f95512e

@mbrookes mbrookes added the component: table This is the name of the generic UI component, not the React module! label Aug 24, 2019
Switch root height to 100% from 100vh
@egerardus egerardus changed the title ScrollableTable demo [docs] ScrollableTable demo Aug 24, 2019
@mbrookes
Copy link
Member

mbrookes commented Aug 24, 2019

@egerardus Thanks for contributing, but I'm not sure I understand the purpose of this PR.

  1. There is already a scrollable table demo.
  2. Pagination and scrolling are alternative methods for navigating long tables. Why both together?

@egerardus
Copy link
Contributor Author

egerardus commented Aug 25, 2019

@mbrookes Thanks for your review. The only scrollable demo I saw was the virtualized table using the additional library dependency.

In short terms:

None of the demos covered any of my use cases fully, this demo would cover most of them:

  • A simple scrollable table with fixed headers that does not require virtualization or additional libraries
  • A sortable, filterable big data table for > 100,000 rows and many columns

To answer your points:

  1. In most cases I use a table that will get a maximum of 100 rows and I would not want to introduce the overhead of virtualization or add another library dependency as shown in the existing scrollable table demo (+30KB to build size in the case of this demo dependency). The standard MUI library works fine for scrolling in most cases. Paging would not be required in these cases obviously.

  2. I am showing them together because that seems to be the most prevalent form of big data UI: a paging table that scrolls some of the rows. I have never seen paging used effectively without some scrolling, e.g. google.com, amazon.com, I could list a few hundred... I am very hard pressed to think of a usable paging table example without any scrolling. The 10% of the tables I use that are not simple scrollable tables are big data tables for enterprise web apps (i.e. greater than 100,000 rows, sortable, filterable, more than 10 columns). I wouldn't want someone to have to page every 10 rows.

By "scrollable" I did not mean "infinite scrolling" which replaces paging entirely, I meant just scrollable as in the table can hold more than the 10 - 15 results that normally fits on desktop or mobile.

In answering your points, I realize I am speaking from my own component use cases, though I do feel they are quite prevalent and common as shown with the two high-traffic sites noted above.
Perhaps a better name and intro for this demo...

Would it communicate better to call this demo a Simple Scrolling Table?

With an introduction:

A sortable and scrollable table with fixed headers. It includes a fixed bottom toolbar that can be used for paging very big data sets.

Essentially this is a demo for a simple scrollable table with fixed headers and a fixed bottom toolbar and without virtualization using just the standard MUI library.

I could remove the paging toolbar if that violates any design maxims but I really feel it would be removing the most commonly used big data component, something that would be useful to have. It's also much easier to take something out of a demo than add it.

Updated the title and information as noted in #17139 (comment)
@mbrookes
Copy link
Member

Thanks for the response. I follow your logic.

Regarding the demo, at the moment it looks like this in Chrome, Firefox and Safari on MacOS 10.14.6:

image

...and doesn't scroll.

@oliviertassinari
Copy link
Member

@egerardus Hold on, what do you think of focusing on the fixed head problem? (#6625)

@egerardus
Copy link
Contributor Author

egerardus commented Aug 26, 2019

Just a few commits later...

Thanks again @mbrookes it's looking good now. Seemed to have an issue with the wrapper needed for the demos, here it is in Chrome v76:

chrome

And here in Firefox v67:

firefox

And here in Chrome for ipad:

image

@oliviertassinari
Copy link
Member

@egerardus Well done!

@egerardus
Copy link
Contributor Author

@oliviertassinari agreed that adding the fixedHeader property is much better than a demo, I will see if I can carve out some time to delve into this.

Though... the comments on #6625 that are in the vain of: "sticky head solution not working for me" offer no reproducible examples, so it could just be an implementation problem in which case a demo would be a useful answer for the immediate future.

@mbrookes
Copy link
Member

@egerardus Much better!

jerry added 2 commits August 26, 2019 19:01
Added class to prevent losing the header bottom border. Going to stop fiddling with this now...
@oliviertassinari
Copy link
Member

@egerardus Ok, let me see if we can abstract some of the logic, also, I will try to simplify the demo to focus on the fixed header problem.

@oliviertassinari oliviertassinari self-assigned this Aug 27, 2019
@egerardus
Copy link
Contributor Author

@oliviertassinari OK sounds good

@oliviertassinari oliviertassinari changed the title [docs] ScrollableTable demo [docs] Add fixed header demo Aug 27, 2019
@oliviertassinari oliviertassinari added the new feature New feature or request label Aug 27, 2019
@oliviertassinari oliviertassinari changed the title [docs] Add fixed header demo [Table] Add sticky header support Aug 27, 2019
@oliviertassinari oliviertassinari removed their assignment Aug 27, 2019
@oliviertassinari
Copy link
Member

@egerardus Could you have a look at the changes? Thanks!

@oliviertassinari
Copy link
Member

I have introduced a new prop to the table component. It sets the position sticky style and a few more. Unfortunately, it doesn't support IE 11. If vertical scrolling is not an issue, people can apply the same solution than the table pagination to the header. However, if they also want to support horizontal scrolling, something often required, and IE 11, then we would need to have a way different API. I believe that we would need to have something close to material-table, ag-grid, Telerik, antd, etc.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 27, 2019

So far, we have been focusing on the styles with the existing table components, in the future, I think that we should explore a higher-level API. The way ag-grid handle the community vs enterprise products is interesting: https://www.ag-grid.com/javascript-grid-set-license/. I think that we could do the same with a @material-ui/pro package and a new repository. We could even consider make the missing tree views feature as our first pro content (drag and drop, checkboxes)

@egerardus
Copy link
Contributor Author

@oliviertassinari You're a wizard, I think it's great.

I need to implement some grid components soon and would like to use material-ui for it. Depending on availability I will work toward doing a PR for higher level "grid" component when I have something stable together.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 28, 2019

@egerardus Thanks you for confirming the need for a table component that goes beyond handling the styles. The next steps ahead are unclear. Have you considered using material-table?

@eps1lon eps1lon merged commit 45d2ce2 into mui:master Aug 28, 2019
@eps1lon
Copy link
Member

eps1lon commented Aug 28, 2019

@egerardus Thank you so much for this. Closing an important issue with the first PR 👍

@egerardus
Copy link
Contributor Author

@oliviertassinari I will check it out now, is the general idea to merge material-table into the core library at some point for this?

If so that is what I would like to contribute to.

@oliviertassinari
Copy link
Member

I have no idea. I don't think that we should close any option until we focus on this problem/opportunity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: table This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Table] Fixing head in material
5 participants