search

iTwin / iTwinUI-react

A react component library for iTwinUI.
https://github.com/iTwin/iTwinUI
Other
83 stars 23 forks source link

feat(Table): First level header now respected #911

Closed adamhock closed 1 year ago

adamhock commented 1 year ago

Table components no longer have to specify a Header. The following basic Table structure has changed

Before: image

After: image

Tables that do specify a Header will have that table header displayed on top of the column headers like so image

Fixes #153

TODO

adamhock commented 1 year ago

I hate to ask this because we're on a time crunch, but I can't tell that this works to fix the problem with stacked/grouped headers. Can you make a storybook example and maybe a unit test?

A Table Header story has been added (if there are any other name suggestions for this story, feel free to chime in!).

Unit tests have also been added

adamhock commented 1 year ago

Have you tested grouped headers scenario described in this issue?

I haven't yet. I'll update PR with TODO

elephantcatdog commented 1 year ago

A Table Header story has been added (if there are any other name suggestions for this story, feel free to chime in!).

Stacked Header? Grouped Header? Detailed Header? Custom Header?

gretanausedaite commented 1 year ago

I haven't yet. I'll update PR with TODO

I added double headers to story to check that: image So I think that should be good (maybe we need to think about styling) @FlyersPh9

gretanausedaite commented 1 year ago

A Table Header story has been added (if there are any other name suggestions for this story, feel free to chime in!).

Stacked Header? Grouped Header? Detailed Header? Custom Header?

'Grouped header' or 'header groups' sounds good to me.

bentleyvk commented 1 year ago

You can take inspiration for the story from the react-table examples: https://react-table-v7.tanstack.com/docs/examples/grouping

adamhock commented 1 year ago

I ended up removing the Grouped Header story and test since the feature isn't entirely flushed out and the goal for this PR is just to fix the first level header bug

mayank99 commented 1 year ago

@adamhock What's the status of this PR? Is your PR tackling both parts of issue? If not, what is your plan to move forward with it?

Also if you're not able to revert the large number of file changes in the diff, I would suggest opening a new PR.

adamhock commented 1 year ago

@adamhock What's the status of this PR? Is your PR tackling both parts of issue? If not, what is your plan to move forward with it?

Also if you're not able to revert the large number of file changes in the diff, I would suggest opening a new PR.

My PR is tackling both parts of the issue. First level Header is no longer required, and when users put a first level Header in their Tables, a table header is shown instead of being ignored.

There's an open comment about Grouped Headers not working for Horizontal Scrolling and Sticky Columns, but I'm not sure if that should just be fixed in another PR so that this one can get merged to fix the major bug.

As for the image changes, I'm stuck on how to revert them, so I will just open a new PR

adamhock commented 1 year ago

Closing this PR and reopening in #935