Conversation
and apply styles depending on the existence of aria-sort, instead of `[aria-disabled="false"]` which is not as self-explaining.
Contributor
Author
Closed
severo
added a commit
that referenced
this pull request
Mar 4, 2026
Since #125, the aria-sort attribute on column headers incorrectly encoded if the column could be sorted. But in the read-only case, the order is set, but no interaction is possible. We add a new 'data-can-sort' attribute to split concerns: - aria-sort recovers its semantics: what is the current order used for the displayed contents - data-can-sort tells if the column header provides interactions for sorting the column It's theoritically a breaking change in the CSS. In reality, I don't expect anyone to rely on it.
severo
added a commit
that referenced
this pull request
Mar 5, 2026
…derByProvider (#458) * no need for useMemo here * [refactor] split orderBy from setOrderBy in context * remove unneeded context providers * remove useless orderBySize * [refactor] move logic from components to OrderByProvider * add TODOs * Update src/contexts/OrderByContext.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix logic and types * fix tests (found by copilot) * fix logic when sortable and orderBy have a contradiction * BREAKING (CSS) move part of the style to new 'data-can-sort' attribute Since #125, the aria-sort attribute on column headers incorrectly encoded if the column could be sorted. But in the read-only case, the order is set, but no interaction is possible. We add a new 'data-can-sort' attribute to split concerns: - aria-sort recovers its semantics: what is the current order used for the displayed contents - data-can-sort tells if the column header provides interactions for sorting the column It's theoritically a breaking change in the CSS. In reality, I don't expect anyone to rely on it. * add a comment about context split while both values are updated at the same time * add a todo * use the same logic in functional header for ⇅ * fix test * make code more explicit * retry, with aria-disabled instead of disabled * add a comment for clarity on the logic * add a test to show priority of 'sortable' over 'orderBy' in case of contradiction --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add 1px to avoid rounding errors when using offsetWidth. An alternative is to use getBoundingClientRect, but maybe it's good enough this way and less resource-consuming (even if it's a micro-optimization).
Also (a bit unrelated to the PR matter): remove
aria-disabled(potentially breaking, even if it had been added recently) and always usearia-sortto style, as it's clearer semantically.Before:
After: