Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DataGrid] Add specific label for linkOperator #3915

Merged
merged 18 commits into from
Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions docs/data/data-grid/filtering/CustomFilterPanelContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ export default function CustomFilterPanelContent() {
size: 'small',
sx: { mt: 'auto' },
},
valueInputProps: {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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"}

required: true,
},
deleteIconProps: {
sx: {
'& .MuiSvgIcon-root': { color: '#d32f2f' },
alexfauquette marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
3 changes: 0 additions & 3 deletions docs/data/data-grid/filtering/CustomFilterPanelContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@ export default function CustomFilterPanelContent() {
size: 'small',
sx: { mt: 'auto' },
},
valueInputProps: {
required: true,
},
deleteIconProps: {
sx: {
'& .MuiSvgIcon-root': { color: '#d32f2f' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ function GridFilterForm(props: GridFilterFormProps) {
)}
>
<InputLabel htmlFor={linkOperatorSelectId} id={linkOperatorSelectLabelId}>
{apiRef.current.getLocaleText('filterPanelOperators')}
{apiRef.current.getLocaleText('filterPanelLinkOperator')}
</InputLabel>
<rootProps.components.BaseSelect
labelId={linkOperatorSelectLabelId}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const GRID_DEFAULT_LOCALE_TEXT: GridLocaleText = {
// Filter panel text
filterPanelAddFilter: 'Add filter',
filterPanelDeleteIconLabel: 'Delete',
filterPanelLinkOperator: '',
Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Member Author

Choose a reason for hiding this comment

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

"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"

image

We can add it to the aria-label of the input, and not show it on top of the input

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

It may look a bit weird if the user changes the variant to filled but the current approach fixes the accessibility.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

filterPanelOperators: 'Operators',
Copy link
Member

Choose a reason for hiding this comment

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

I don't know why it's in plural but it refers to the current value. The same for "Columns".

Suggested change
filterPanelOperators: 'Operators',
filterPanelOperators: 'Operator',

filterPanelOperatorAnd: 'And',
filterPanelOperatorOr: 'Or',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/arSD.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const arSDGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'اضف فلتر',
filterPanelDeleteIconLabel: 'حذف',
filterPanelLinkOperator: '',
filterPanelOperators: 'العاملين',
filterPanelOperatorAnd: 'و',
filterPanelOperatorOr: 'او',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/bgBG.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const bgBGGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Добави Филтър',
filterPanelDeleteIconLabel: 'Изтрий',
filterPanelLinkOperator: '',
filterPanelOperators: 'Оператори',
filterPanelOperatorAnd: 'И',
filterPanelOperatorOr: 'Или',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/csCZ.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const csCZGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Přidat filtr',
filterPanelDeleteIconLabel: 'Odstranit',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operátory',
filterPanelOperatorAnd: 'A',
filterPanelOperatorOr: 'Nebo',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/daDK.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const daDKGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Tilføj filter',
filterPanelDeleteIconLabel: 'Slet',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operatorer',
// filterPanelOperatorAnd: 'And',
// filterPanelOperatorOr: 'Or',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/deDE.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const deDEGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Filter hinzufügen',
filterPanelDeleteIconLabel: 'Löschen',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operatoren',
filterPanelOperatorAnd: 'Und',
filterPanelOperatorOr: 'Oder',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/elGR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const elGRGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Προσθήκη φίλτρου',
filterPanelDeleteIconLabel: 'Διαγραφή',
filterPanelLinkOperator: '',
filterPanelOperators: 'Τελεστές',
filterPanelOperatorAnd: 'Καί',
filterPanelOperatorOr: 'Ή',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/esES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const esESGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Agregar filtro',
filterPanelDeleteIconLabel: 'Borrar',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operadores',
filterPanelOperatorAnd: 'Y',
filterPanelOperatorOr: 'O',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/faIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const faIRGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'افزودن فیلتر',
filterPanelDeleteIconLabel: 'حذف',
filterPanelLinkOperator: '',
filterPanelOperators: 'عملگرها',
filterPanelOperatorAnd: 'و',
filterPanelOperatorOr: 'یا',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/fiFI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const fiFIGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Lisää suodatin',
filterPanelDeleteIconLabel: 'Poista',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operaattorit',
filterPanelOperatorAnd: 'Ja',
filterPanelOperatorOr: 'Tai',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/frFR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const frFRGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Ajouter un filtre',
filterPanelDeleteIconLabel: 'Supprimer',
filterPanelLinkOperator: '',
filterPanelOperators: 'Opérateurs',
filterPanelOperatorAnd: 'Et',
filterPanelOperatorOr: 'Ou',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/heIL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const heILGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'הוסף מסנן',
filterPanelDeleteIconLabel: 'מחק',
filterPanelLinkOperator: '',
filterPanelOperators: 'אופרטור',
filterPanelOperatorAnd: 'וגם',
filterPanelOperatorOr: 'או',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/itIT.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const itITGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Aggiungi un filtro',
filterPanelDeleteIconLabel: 'Rimuovi',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operatori',
filterPanelOperatorAnd: 'E (and)',
filterPanelOperatorOr: 'O (or)',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/jaJP.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const jaJPGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'フィルター追加',
filterPanelDeleteIconLabel: '削除',
filterPanelLinkOperator: '',
filterPanelOperators: 'オペレータ',
filterPanelOperatorAnd: 'And',
filterPanelOperatorOr: 'Or',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/koKR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const koKRGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: '필터 추가',
filterPanelDeleteIconLabel: '삭제',
filterPanelLinkOperator: '',
filterPanelOperators: '연산자',
filterPanelOperatorAnd: '그리고',
filterPanelOperatorOr: '또는',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/nlNL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const nlNLGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Filter toevoegen',
filterPanelDeleteIconLabel: 'Verwijderen',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operatoren',
filterPanelOperatorAnd: 'En',
filterPanelOperatorOr: 'Of',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/plPL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const plPLGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Dodaj filtr',
filterPanelDeleteIconLabel: 'Usuń',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operator',
filterPanelOperatorAnd: 'I',
filterPanelOperatorOr: 'Lub',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/ptBR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const ptBRGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Adicionar filtro',
filterPanelDeleteIconLabel: 'Excluir',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operadores',
filterPanelOperatorAnd: 'E',
filterPanelOperatorOr: 'Ou',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/ruRU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const ruRUGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Добавить фильтр',
filterPanelDeleteIconLabel: 'Удалить',
filterPanelLinkOperator: '',
filterPanelOperators: 'Операторы',
filterPanelOperatorAnd: 'И',
filterPanelOperatorOr: 'Или',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/skSK.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const skSKGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Pridať filter',
filterPanelDeleteIconLabel: 'Odstrániť',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operátory',
filterPanelOperatorAnd: 'A',
filterPanelOperatorOr: 'Alebo',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/trTR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const trTRGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Filtre Ekle',
filterPanelDeleteIconLabel: 'Kaldır',
filterPanelLinkOperator: '',
filterPanelOperators: 'Operatör',
filterPanelOperatorAnd: 'Ve',
filterPanelOperatorOr: 'Veya',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/ukUA.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const ukUAGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Додати фільтр',
filterPanelDeleteIconLabel: 'Видалити',
filterPanelLinkOperator: '',
filterPanelOperators: 'Оператори',
filterPanelOperatorAnd: 'І',
filterPanelOperatorOr: 'Або',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/viVN.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const viVNGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: 'Thêm bộ lọc',
filterPanelDeleteIconLabel: 'Xóa',
filterPanelLinkOperator: '',
filterPanelOperators: 'Toán tử',
filterPanelOperatorAnd: 'Và',
filterPanelOperatorOr: 'Hoặc',
Expand Down
1 change: 1 addition & 0 deletions packages/grid/_modules_/grid/locales/zhCN.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const zhCNGrid: Partial<GridLocaleText> = {
// Filter panel text
filterPanelAddFilter: '添加筛选器',
filterPanelDeleteIconLabel: '删除',
filterPanelLinkOperator: '',
filterPanelOperators: '操作器',
filterPanelOperatorAnd: '与',
filterPanelOperatorOr: '或',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface GridLocaleText {
// Filter panel text
filterPanelAddFilter: React.ReactNode;
filterPanelDeleteIconLabel: string;
filterPanelLinkOperator: string;
filterPanelOperators: React.ReactNode;
filterPanelOperatorAnd: React.ReactNode;
filterPanelOperatorOr: React.ReactNode;
Expand Down
2 changes: 1 addition & 1 deletion scripts/performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ async function testFilter100kRows(page) {
await page.goto(baseUrl);

await page.selectOption('label=Columns', 'currencyPair');
await page.selectOption('label=Operators >> nth=1', 'startsWith');
await page.selectOption('label=Operators', 'startsWith');
const t0 = await page.evaluate(() => performance.now());
await page.type('label=Value', 'usd');

Expand Down