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

[DataTableSkeleton] Add prop to hide header in table skeleton #5993

Closed
robherley opened this issue Apr 30, 2020 · 10 comments · Fixed by #6192
Closed

[DataTableSkeleton] Add prop to hide header in table skeleton #5993

robherley opened this issue Apr 30, 2020 · 10 comments · Fixed by #6192

Comments

@robherley
Copy link

Summary

After #5752 was merged, the DataTableSkeleton component always has a header now, which is a change from how it used to look previously (which is understandable, given the v10 bump).

For the React component, it would be nice to have a prop such as showHeader that can show or hide the extra divs, instead of having to apply some sort of CSS to hide them.

@asudoh
Copy link
Contributor

asudoh commented Apr 30, 2020

Sounds close to #5967 - CC @carbon-design-system/design

@robherley
Copy link
Author

Just to clarify, it's not the header row, it's the new header for the table (container with title & description)

@nictownsend
Copy link
Contributor

This is a breaking change that was not communicated in the release changelog... I'm now forced to override with CSS behaviour that has been forced rather than added as an optional.

@nictownsend
Copy link
Contributor

And it's definitely not a backwards compatible change, so by semver should have been a major version bump.

@nictownsend
Copy link
Contributor

I'm happy to look at raising a PR if it's acceptable for it to be customisable

@asudoh
Copy link
Contributor

asudoh commented May 4, 2020

@nictownsend My apologies for the change in table skeleton yielded a minor breaking change; The change was meant to be part of v10 but the team slipped it through the crack, and it was seen not worth waiting for v11. A possible future consideration for this area is better release noting.

Wrt providing an option to hide title/description portion, technically it makes sense, but let's see what @carbon-design-system/design thinks from the point of whether our design guide since v10 allows it.

@nictownsend
Copy link
Contributor

Thank you - for now, the DataTable + DataTable skeleton are not consistent; I use a DataTable without a header or toolbar, and I will have to use CSS to hide the Skeleton header and toolbar for consistency.

If design do not believe the guide for v10 allows a DataTable Skeleton without header/toolbar, then does that also mean the DataTable component (https://www.carbondesignsystem.com/components/data-table/usage) needs updating in the guide to state a Toolbar must always exist?

@richardpilot
Copy link

There are also use cases where you want the DataTable to be a skeleton but you have actions available in the toolbar that do not need a skeleton. For example, the primary button of a table being create new object, which can still occur whilst the underlying data is being refresh (causing a skeleton) e.g.

image

@GraceeFlower
Copy link

There are also use cases where you want the DataTable to be a skeleton but you have actions available in the toolbar that do not need a skeleton. For example, the primary button of a table being create new object, which can still occur whilst the underlying data is being refresh (causing a skeleton) e.g.

image

Hey team, is this question be resolved? I also wanna keep the search bar shown in skeleton status, any guidance? Thanks!

@tw15egan
Copy link
Collaborator

@GraceeFlower, there hasn't been any more work on this item since the last comment. I think keeping the search bar visible may be a separate enhancement, so I'd create a new issue as a Feature Request and we can track it that way. If you have any time to dedicate to adding in this functionality to the repo, we would welcome a PR if the request is approved by design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants