-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat(Table): new colors on zebra and hover #3147
Conversation
🦋 Changeset detectedLatest commit: 47bbb49 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Preview deployments for this pull request: Storybook - |
Coverage Report
File CoverageNo changed files found. |
@@ -12,7 +12,7 @@ | |||
--dsc-table-divider-border-width: calc(var(--ds-border-width-default) + 1px); | |||
--dsc-table-header-background--hover: var(--ds-color-surface-tinted); | |||
--dsc-table-header-background--sorted: var(--ds-color-background-tinted); | |||
--dsc-table-header-background: var(--ds-color-background-default); | |||
--dsc-table-header-background: transparent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest keep it transparent for now and then add a background later if someone has a problem with it. Add a section in the docs about needing to add a background color on thead
that matches your background color if you plan on using stickyHeader
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, it makes no sense for us to release an unfinished feature.
I'd rather add a bakground only to the header, or remove the sticky header feature completely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jepp, as discussed on daily 17-18 feb. Lets add a background-default
color as default, since you either way have to change a background color if it does not match, this way it may be ok for most people, and they don't need to change the color.
Co-authored-by: Michael Marszalek <mimarz@gmail.com>
resolves #2828