fix(DataGrid): Single select should not render header selection indicator #33752
Conversation
This file contains 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
fabricteam
reviewed
Jan 31, 2025
| @@ -25,6 +25,7 @@ export const useDataGridRow_unstable = (props: DataGridRowProps, ref: React.Ref< | |||
| const selected = useDataGridContext_unstable(ctx => ctx.selection.isRowSelected(rowId)); | |||
There was a problem hiding this comment.
🕵🏾♀️ visual regressions to review in the fluentuiv9 Visual Regression Report
Avatar Converged 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| Avatar Converged.Badge Mask RTL.chromium.png | 8 | Changed |
| Avatar Converged.badgeMask.chromium.png | 3 | Changed |
DataGridConverged - subtle multi select 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| DataGridConverged - subtle multi select.default.chromium.png | 0 | Added |
DataGridConverged - subtle single select 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| DataGridConverged - subtle single select.default.chromium.png | 0 | Added |
ling1726
force-pushed
the
react-table/fix/data-grid-single-header
branch
from
919493c to
97a00c7
Compare
ling1726
changed the title
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
This reverts commit 40ab46b.
layershifter
reviewed
Feb 3, 2025
| @@ -183,6 +183,20 @@ describe('DataGridRow', () => { | |||
| expect(toggleRow).toHaveBeenCalledTimes(0); | |||
| }); | |||
|
|
|||
| it('should disable selection cell input if the row is in a header and the selection mode is singe', () => { | |||
There was a problem hiding this comment.
Suggested change
| it('should disable selection cell input if the row is in a header and the selection mode is singe', () => { | |
| it('should disable selection cell input if the row is in a header and the selection mode is single', () => { |
🐱
layershifter
reviewed
Feb 3, 2025
Comment on lines
+57
to
+59
| const isHeader = useIsInTableHeader(); | ||
| const isSingleSelect = useDataGridContext_unstable(ctx => ctx.selection.selectionMode === 'single'); | ||
| const isSubtle = useDataGridContext_unstable(ctx => ctx.subtleSelection); |
There was a problem hiding this comment.
IMO this should be moved to state as in other components
There was a problem hiding this comment.
If this is moved to state, the only purpose would be to increase our API surface, and pass through to this hook where it is used.
Since styles hooks are hooks, what's the consequence?
layershifter
reviewed
Feb 3, 2025
As discussed these changes are for DataGrid - updated the PR title to reflect this |
ling1726
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Creates specific overrides for subtle selection in the DataGridRow styles and makes the radio input in the header disabled so that users cannot navigate to it with keyboard or screen reader
Fixes #33667
Fixes #33561