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

Rename Table's initialSortedRow to initialSortedColumn #2491

Merged

Conversation

Burtchen
Copy link
Contributor

@Burtchen Burtchen commented Apr 9, 2024

Discovered during the implementation of other table-related functionalities: The language/variables sometimes confuse row for column, which... well, it's confusing. Let's fix that.

I went for a straightforward change (instead of a deprecation) in the TableHead component. While it is exposed, none of the internal projects seem to use it and our Storybook also does not showcase it as a 'proper' component.

For the Table itself, the initialSortedRow prop is now deprecated in lieu of initialSortedColumn

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests
  • Meets minimum browser support
  • Meets accessibility requirements

Copy link

changeset-bot bot commented Apr 9, 2024

🦋 Changeset detected

Latest commit: 577f712

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sumup/circuit-ui Minor
@sumup/eslint-plugin-circuit-ui Major

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

Copy link

vercel bot commented Apr 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
oss-circuit-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 15, 2024 9:12am

Copy link

codecov bot commented Apr 9, 2024

Codecov Report

Attention: Patch coverage is 95.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 87.69%. Comparing base (3c2763c) to head (577f712).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2491   +/-   ##
=======================================
  Coverage   87.68%   87.69%           
=======================================
  Files         198      198           
  Lines       21209    21221   +12     
  Branches     1281     1285    +4     
=======================================
+ Hits        18597    18609   +12     
  Misses       2560     2560           
  Partials       52       52           
Files Coverage Δ
...omponents/Table/components/TableHead/TableHead.tsx 100.00% <100.00%> (ø)
packages/circuit-ui/components/Table/utils.ts 93.54% <100.00%> (ø)
packages/circuit-ui/components/Table/Table.tsx 87.12% <93.33%> (+0.53%) ⬆️

@Burtchen Burtchen marked this pull request as ready for review April 11, 2024 11:15
@Burtchen Burtchen requested a review from a team as a code owner April 11, 2024 11:15
@Burtchen Burtchen requested review from connor-baer and removed request for a team April 11, 2024 11:15
Copy link
Contributor

@amelako amelako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹🎉

packages/circuit-ui/components/Table/Table.tsx Outdated Show resolved Hide resolved
.changeset/slimy-carpets-tease.md Outdated Show resolved Hide resolved
@connor-baer connor-baer changed the title change initialsortedrow to initialsortedcolumn for the table props Rename Table's initialSortedRow to initialSortedColumn Apr 15, 2024
@connor-baer connor-baer merged commit 35ee26a into main Apr 15, 2024
11 checks passed
@connor-baer connor-baer deleted the bugfix/change-initialsortedrow-to-initialsortedcolumn branch April 15, 2024 09:23
@connor-baer connor-baer mentioned this pull request Apr 15, 2024
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.

3 participants