[DataGridPro] Allow multi sorting without modifier key #17925
Conversation
cherniavskii
added
scope: data grid
|
Deploy preview: https://deploy-preview-17925--material-ui-x.netlify.app/ Updated pages: |
| * If `true`, the multi-sorting is applied on column header click without modifier key. | ||
| * @default false | ||
| */ | ||
| multiSortingWithoutModifier: PropTypes.bool, |
There was a problem hiding this comment.
Name of the prop holds the implementation detail.
Maybe it can be just multiSorting or something like keepSort
There was a problem hiding this comment.
We already have the disableMultipleColumnsSorting that enables or not the whole multi-sorting feature.
I fear that multiSorting would create confusion between the two.
Maybe we could combine both at some point (next major though).
And if we keep 2 distincts props, the main goal for me would be to make the distinction between the two crystal clear
There was a problem hiding this comment.
Name of the prop holds the implementation detail.
TBH, this is intentional to make it clear for devs that multi sorting will work without the modifier key 😅
Maybe it can be just multiSorting or something like keepSort
But multisorting is enabled by default, it's just that the default behavior requires users to use the modifier key.
Hence the naming.
There was a problem hiding this comment.
maybe then multiSortingOnClick to say when it is working instead of what do you not need 😁
There was a problem hiding this comment.
Maybe we could combine both at some point (next major though).
I prefer to avoid another breaking change for such a small feature.
I think 2 distinct props are OK.
Initially, I called it multipleColumnsSortingWithoutModifier, but I thought it's too long 😅
How about this?
multiSortingMode: 'withModifierKey' | 'withoutModifierKey'There was a problem hiding this comment.
multiSortingMode: 'always' | 'withModifierKey'
There was a problem hiding this comment.
Ok, this looks good to me
There was a problem hiding this comment.
multiSortingMode looks good to me
There was a problem hiding this comment.
Should the naming be consistent though to link them together: disableMultipleColumnsSorting and multipleColumnsSortingMode?
And if we are convinced that it's too verbose and "multiSorting" is enough to convey the intention, they could become disableMultiSorting and multiSortingMode in the next major.
Sidenote: I think it also makes sense to make "always" the default behavior in the next major.
There was a problem hiding this comment.
Should the naming be consistent though to link them together:
disableMultipleColumnsSortingandmultipleColumnsSortingMode?
Agree with this
I see that we also have disableMultipleColumnsFiltering
Closes #14563