[DataGrid] Add specific label for linkOperator #3915
Conversation
|
These are the results for the performance tests:
|
| @@ -41,6 +41,7 @@ export const GRID_DEFAULT_LOCALE_TEXT: GridLocaleText = { | |||
| // Filter panel text | |||
| filterPanelAddFilter: 'Add filter', | |||
| filterPanelDeleteIconLabel: 'Delete', | |||
| filterPanelLinkOperator: '', | |||
There was a problem hiding this comment.
We need to add a label. Keeping it empty violates https://www.w3.org/TR/using-aria/#fifthrule
The correct name would be "logic operator" but it's too long. A benchmark might be necessary. For me, "operator" is perfect and "operators" (referring to those like "contains") should be changed to "rule" or "condition". But if we change then we lose sync with the translation constant. Anyway, the API uses some confusing names for the filter model: #1722 (comment)
There was a problem hiding this comment.
"logic operator" seems good. From what I see in other tables, they nether display the label (And/Or) is clear enough, except notion which adds an helper: "All filters much match"
We can add it to the aria-label of the input, and not show it on top of the input
There was a problem hiding this comment.
Removing the label may not be ideal, there's no demo without it. We could experiment with the ToggleButton. It doesn't require a visible label. There's also a demo using a Menu.
There was a problem hiding this comment.
I don't think ToggleButton is a good option. We already have a lot of information on the page. Showing both "And" and "Or" options will add confusion.
About the Menu demo, I don't see the difference.
The demo is using a combination of aria-label and aria-labelledby to compensate the missing label, plus some role: 'listbox', aria-haspopup="listbox", ... to respect a11y.
The current solution add aria-label to compensate the missing label, and we use meaning full HTML (select and option) to let the screen reader understand the structure.
If your point is that selects accessibility does not contain demo without label, I think the topic of this section is mostly how to add a label on Select component
There was a problem hiding this comment.
It may look a bit weird if the user changes the variant to filled but the current approach fixes the accessibility.
There was a problem hiding this comment.
In such a case it would be nice to have a way to add a label, but I do not see how to make that easy to find for developers.
What do you think about wanting to see if some users require to display the label?
| @@ -41,6 +41,7 @@ export const GRID_DEFAULT_LOCALE_TEXT: GridLocaleText = { | |||
| // Filter panel text | |||
| filterPanelAddFilter: 'Add filter', | |||
| filterPanelDeleteIconLabel: 'Delete', | |||
| filterPanelLinkOperator: '', | |||
| filterPanelOperators: 'Operators', | |||
There was a problem hiding this comment.
I don't know why it's in plural but it refers to the current value. The same for "Columns".
| filterPanelOperators: 'Operators', | |
| filterPanelOperators: 'Operator', |
| @@ -65,9 +65,6 @@ export default function CustomFilterPanelContent() { | |||
| size: 'small', | |||
| sx: { mt: 'auto' }, | |||
| }, | |||
| valueInputProps: { | |||
There was a problem hiding this comment.
Is it possible to make all fields in the filter panel outlined?
I was playing with it a bit and wasn't able to make value input outlined.
There was a problem hiding this comment.
Not easily, because the value input component can be any thing developer wants.
By default, we set variant="standard" in GridFilterInputValue
https://github.com/alexfauquette/material-ui-x/blob/refacto-export-button/packages/grid/_modules_/grid/components/panel/filterPanel/GridFilterInputValue.tsx#L128:L128
To customize them, developers can edit operators. To make them outlined thy should add to the default operators InputComponentProps: {variant: "outlined"}
There was a problem hiding this comment.
The "margin" between the logic operators and the columns select is missing. Actually it's not a margin but only the select not taking the full width. Now we need to add a margin to compensate.
In https://deploy-preview-3915--material-ui-x.netlify.app/components/data-grid/
In https://mui.com/components/data-grid/demo/
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
removed
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
added
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
removed
the
PR: out-of-date
Co-authored-by: Matheus Wichman <[email protected]>
oliviertassinari
added
the
component: data grid
Fix #4061
Preview: https://deploy-preview-3915--material-ui-x.netlify.app/components/data-grid/
The demo has two problem:
Pb 1
The link operator has the label "Operators". Thius bug is here from a long time ago. I create a key for linkOperator, and let it empty in all languages, such that dev can customize it.
Pb 2
passing required to the input value does not show the star, so I simply removed it from the demo