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

fix(react): update data-table based on audit #10966

Merged
merged 5 commits into from
Mar 22, 2022

Conversation

joshblack
Copy link
Contributor

@joshblack joshblack commented Mar 11, 2022

Reference #10413

This PR includes bugfixes identified in #10413

Changelog

New

Changed

  • Update Data Table styles to reflect open issues found in audit

Removed

Testing / Reviewing

  • Verify changes in audit are reflected in this PR

@joshblack
Copy link
Contributor Author

joshblack commented Mar 11, 2022

Changes

  • Toolbar
    • .#{$prefix}--toolbar-search-container-expandable.#{$prefix}--search--disabled .#{$prefix}--search-magnifier-icon background color should be just $layer
    • The hover on the settings overflow menu is not showing up correctly. Should be using $layer-hover not $background-hover
    • .#{$prefix}--batch-summary__para update typestyle to $body-compact-01
  • Core
    • .#{$prefix}--data-table-header__title update type style to $heading-03
    • .#{$prefix}--data-table-header__description update type style toe $body-compact-01
    • // First row tr.#{$prefix}--data-table--selected:first-of-type td border color should be $border-subtle-selected
    • // Radio border-bottom should be $border-subtle-selected not $layer-active
  • Expandable table
    • .#{$prefix}--table-expand__svg should be $icon-primary not v10 $ui-05
    • borders and box-shadows using $layer-active is incorrect and should be using $border-subtle-selected instead.
  • Storybook
    • In the "Batch action / Sm" story the table rows should be changing size along with the action bar.
      • Note: this problems seems storybook related and will sometimes work and sometimes not work, will keep an eye on this

@joshblack
Copy link
Contributor Author

@aagonzales tried to capture as many of the changes you listed as I could in: #10966 (comment)

Let me know if I misinterpreted any of them in the changes!

The only question I had was around the zebra stripe colors, I couldn't find this in the Sketch file and wanted to confirm what token should be used for the background of zebra strikes.

@netlify
Copy link

netlify bot commented Mar 11, 2022

✅ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: 0fa7c5c

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/623a16eec722840008feb331

😎 Browse the preview: https://deploy-preview-10966--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Mar 11, 2022

✅ Deploy Preview for carbon-components-react ready!
Built without sensitive environment variables

🔨 Explore the source changes: 0fa7c5c

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/623a16eeff042f00087eec7e

😎 Browse the preview: https://deploy-preview-10966--carbon-components-react.netlify.app

@netlify
Copy link

netlify bot commented Mar 11, 2022

✅ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 0fa7c5c

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/623a16eeef319d0008311ec0

😎 Browse the preview: https://deploy-preview-10966--carbon-elements.netlify.app

@aagonzales
Copy link
Member

Seems to all be good I think

@joshblack joshblack marked this pull request as ready for review March 21, 2022 15:52
@joshblack joshblack requested a review from a team as a code owner March 21, 2022 15:52
@joshblack joshblack requested review from tay1orjones and dakahn March 21, 2022 15:52
@joshblack joshblack requested a review from aagonzales March 21, 2022 15:52
@tay1orjones
Copy link
Member

@joshblack I'm only seeing 3 files changed, with no changes to type styles or other changes listed in the comments above?

@joshblack
Copy link
Contributor Author

@tay1orjones I think they were handled by TJ before he went on leave! Sorry for the mistake on my end, should have not copied over the whole checklist under the "changed" section since I only tweaked part of this.

@tay1orjones
Copy link
Member

@joshblack Ah! Yes that makes sense then. Thanks

@kodiakhq kodiakhq bot merged commit 118938a into carbon-design-system:main Mar 22, 2022
tay1orjones pushed a commit to tay1orjones/carbon that referenced this pull request Mar 23, 2022
)

* chore: check-in work

* fix(styles): update search bar with data table

* docs(data-table): update batch actions > small story

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
kodiakhq bot added a commit that referenced this pull request Mar 23, 2022
* refactor(project): update AC config and add in AVT tests (#10956)

* test(react): add AVT to breadcrumb

* feat(jest-config): update accessibility-checker config

* test(react): add accessibility-checker tests to ContentSwitcher

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* fix(Combobox): compose onStateChange event handlers (#10968)

* fix(Combobox): compose onStateChange event handlers

* fix(Combobox): remove unnecessary short circuiting

* fix(Combobox): get the optional chaining operator syntax right

* fix(Combobox): add conditional check to downshiftProps object as well

* chore(Carbon): update public api snapshot

* fix(deps): update dependency @carbon/telemetry to v0.1.0 (#10982)

Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Josh Black <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* chore(DataTable): size prop audit (#10999)

* chore(DataTable): size prop audit first draft

* chore(DataTable): removed zombies

* chore(DataTable): removed v10 styles

* chore(DataTable): excluded v10 stories

* chore(DataTable): removed sm story from Toolbar

* chore(DataTable): included more dynamic and expansion stories

* refactor(react): update components to usePrefix or PrefixContent (#11015)

* refactor(react): update components to usePrefix or PrefixContent

* refactor(react): update components to usePrefix (2)

* fix(React): update to new prefix method

* refactor(react): update components to usePrefix (3)

* refactor(react): update components to usePrefix (4)

* chore(react): revert useNormalizedProps as it is used in classes

* chore(react): update tests and incorrect import paths

* test(react): update tests based on changes

* refactor(react): update TabContent usePrefix usage

Co-authored-by: D.A. Kahn <[email protected]>

* 10957 fix datepicker unknown prop warning (#10959)

* fix(Datepicker): add onOpen to destructured props

* fix(Datepicker): remove render call for onOpen

* fix(react): update data-table based on audit (#10966)

* chore: check-in work

* fix(styles): update search bar with data table

* docs(data-table): update batch actions > small story

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

* test: update snap

* chore(project): update action to no longer use v10 branch ref

Co-authored-by: Josh Black <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: DAK <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Scott Strubberg <[email protected]>
Co-authored-by: D.A. Kahn <[email protected]>
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.

4 participants