Skip to content

fix: update DataTable sorting for zero values - #7842

Merged
joshblack merged 1 commit into
mainfrom
investigate-datatable-number-sorting
May 15, 2026
Merged

fix: update DataTable sorting for zero values#7842
joshblack merged 1 commit into
mainfrom
investigate-datatable-number-sorting

Conversation

@joshblack

Copy link
Copy Markdown
Member

Changelog

New

N/A

Changed

  • DataTable now treats 0 as a populated value when sorting numeric columns.

Removed

N/A

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

  • npx vitest run packages/react/src/DataTable/__tests__/DataTable.test.tsx
  • npm run type-check

Merge checklist

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 15, 2026 15:28
@joshblack
joshblack requested a review from a team as a code ownerMay 15, 2026 15:28
@joshblack
joshblack requested a review from liuliu-devMay 15, 2026 15:28
@changeset-bot

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 00f21af

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

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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

@github-actionsgithub-actionsBot added staff Author is a staff member integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels May 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@joshblackjoshblack changed the title Fix DataTable sorting for zero valuesfix: update DataTable sorting for zero valuesMay 15, 2026

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a DataTable sorting bug where numeric 0 values were treated as blank (because of a truthiness check), causing them to sort alongside null/undefined/'' instead of being ordered as numeric values.

Changes:

  • Replace truthy checks on sort values with an explicit isBlankValue helper that only treats null/undefined/'' as blank.
  • Update existing sort test to include a 0 row and add a new test asserting 0 sorts as a numeric value in both directions.
  • Add a patch changeset.
Show a summary per file
FileDescription
packages/react/src/DataTable/useTable.tsIntroduce isBlankValue and use it in the sort comparator instead of truthy checks.
packages/react/src/DataTable/tests/DataTable.test.tsxExtend existing sort test with a 0 row and add a dedicated test for zero values.
.changeset/datatable-zero-sorting.mdPatch changeset describing the fix.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

@joshblackjoshblack added the Canary Release Apply this label when you want CI to create a canary release of the current PR label May 15, 2026
@primer-integration

Copy link
Copy Markdown

👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/20875

@primer-integration

Copy link
Copy Markdown

Integration test results from github/github-ui:

Passed CI  Passed
Passed VRT  Passed
Passed Projects  Passed

All checks passed!

@joshblack
joshblack added this pull request to the merge queueMay 15, 2026
Merged via the queue into main with commit 635357eMay 15, 2026
66 checks passed
@joshblack
joshblack deleted the investigate-datatable-number-sorting branch May 15, 2026 19:21
@primerprimerBot mentioned this pull request May 15, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canary ReleaseApply this label when you want CI to create a canary release of the current PRintegration-tests: recommendedThis change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpmstaffAuthor is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@joshblack@liuliu-dev