From 165e007a599c5d0a91b4fc355d1ce521c16dc862 Mon Sep 17 00:00:00 2001 From: Titani Date: Thu, 8 Aug 2024 17:25:49 -0400 Subject: [PATCH 1/9] fix(Table deprecated): Fix editable select row to close menu when click outside --- .../cypress/integration/tableeditable.spec.ts | 10 +++---- .../demos/TableDemo/TableEditableDemo.tsx | 19 ++++++++---- .../Table/EditableSelectInputCell.tsx | 9 ++++-- .../components/Table/examples/Table.md | 29 ++++++++++--------- 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/react-integration/cypress/integration/tableeditable.spec.ts b/packages/react-integration/cypress/integration/tableeditable.spec.ts index 9271bcd66e2..dce45244d4d 100644 --- a/packages/react-integration/cypress/integration/tableeditable.spec.ts +++ b/packages/react-integration/cypress/integration/tableeditable.spec.ts @@ -1,4 +1,4 @@ -xdescribe('Table Simple Test', () => { +describe('Table Simple Test', () => { it('Navigate to demo section', () => { cy.visit('http://localhost:3000/table-editable-demo-nav-link'); }); @@ -20,9 +20,9 @@ xdescribe('Table Simple Test', () => { '.pf-m-inline-editable > [data-label="Text input col 1"] > .pf-v6-c-inline-edit__input > .pf-v6-c-form-control > input' ).type('test'); cy.get( - '.pf-m-inline-editable > [data-label="Dropdown col 5"] > .pf-v6-c-inline-edit__input > .pf-v6-c-select > .pf-v6-c-select__toggle > .pf-v6-c-select__toggle-button' + '.pf-m-inline-editable > [data-label="Dropdown col 5"] > .pf-v6-c-inline-edit__input > .pf-v6-c-menu-toggle > .pf-v6-c-menu-toggle__controls' ).click(); - cy.get(':nth-child(4) > .pf-v6-c-check__label').click(); + cy.get('#uniqueIdRow1Cell5Option3').click(); cy.get( '.pf-m-inline-editable > .pf-v6-c-table__inline-edit-action > .pf-v6-c-inline-edit__group > :nth-child(1) > .pf-v6-c-button' ).click(); @@ -46,9 +46,9 @@ xdescribe('Table Simple Test', () => { .clear() .type('xyz'); cy.get( - '.pf-m-inline-editable:nth-of-type(2) > [data-label="Dropdown col 5"] > .pf-v6-c-inline-edit__input > .pf-v6-c-select > .pf-v6-c-select__toggle > .pf-v6-c-select__toggle-button' + '.pf-m-inline-editable > [data-label="Dropdown col 5"] > .pf-v6-c-inline-edit__input > .pf-v6-c-menu-toggle > .pf-v6-c-menu-toggle__controls' ).click(); - cy.get('#uniqueIdRow2Cell5Option3 > .pf-v6-c-select__menu-item').click(); + cy.get('#uniqueIdRow2Cell5Option3').click(); cy.get( '.pf-m-inline-editable:nth-of-type(2) > .pf-v6-c-table__inline-edit-action > .pf-v6-c-inline-edit__group > :nth-child(2) > .pf-v6-c-button' ).click(); diff --git a/packages/react-integration/demo-app-ts/src/components/demos/TableDemo/TableEditableDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/TableDemo/TableEditableDemo.tsx index 15963c692c2..e7908dfc72c 100644 --- a/packages/react-integration/demo-app-ts/src/components/demos/TableDemo/TableEditableDemo.tsx +++ b/packages/react-integration/demo-app-ts/src/components/demos/TableDemo/TableEditableDemo.tsx @@ -138,10 +138,12 @@ export class TableEditableDemo extends React.Component { clearSelection={this.clearSelection} isOpen={updatedProps.isSelectOpen} options={(updatedProps as any).options.map((option: any, index: number) => ( - + + {option.value} + ))} - onToggle={(event: any) => { - this.onToggle(event); + onToggle={(_event: any) => { + this.onToggle(rowIndex, cellIndex); }} selections={updatedProps.selected} /> @@ -249,10 +251,12 @@ export class TableEditableDemo extends React.Component { clearSelection={this.clearSelection} isOpen={updatedProps.isSelectOpen} options={(updatedProps as any).options.map((option: any, index: number) => ( - + + {option.value} + ))} onToggle={(_event: any) => { - this.onToggle(_event); + this.onToggle(rowIndex, cellIndex); }} selections={updatedProps.selected} /> @@ -362,6 +366,7 @@ export class TableEditableDemo extends React.Component { newCellProps.editableValue = newSelected; newCellProps.selected = newSelected; + newCellProps.isSelectOpen = false; } this.setState({ @@ -369,8 +374,10 @@ export class TableEditableDemo extends React.Component { }); }; - onToggle = (_event: any) => { + onToggle = (rowIndex: string | number | undefined, cellIndex: string | number | undefined) => { const newRows = Array.from(this.state.rows); + newRows[rowIndex as number].cells[cellIndex].props.isSelectOpen = + !newRows[rowIndex as number].cells[cellIndex].props.isSelectOpen; this.setState({ rows: newRows }); diff --git a/packages/react-table/src/components/Table/EditableSelectInputCell.tsx b/packages/react-table/src/components/Table/EditableSelectInputCell.tsx index e139f3b1524..038f159c890 100644 --- a/packages/react-table/src/components/Table/EditableSelectInputCell.tsx +++ b/packages/react-table/src/components/Table/EditableSelectInputCell.tsx @@ -47,6 +47,8 @@ export const EditableSelectInputCell: React.FunctionComponent { + const [isSelectOpen, setIsSelectOpen] = React.useState(isOpen); + const onSelectHandler = ( event: React.MouseEvent | React.ChangeEvent, newValue: any | any[] @@ -64,11 +66,12 @@ export const EditableSelectInputCell: React.FunctionComponent setIsSelectOpen(isOpen)} selected={selections} toggle={(toggleRef: any) => ( - - {isOpen ? 'Expanded' : 'Collapsed'} + + {isSelectOpen ? 'Expanded' : 'Collapsed'} )} > diff --git a/packages/react-table/src/deprecated/components/Table/examples/Table.md b/packages/react-table/src/deprecated/components/Table/examples/Table.md index e8fd613ec9c..213438f34ce 100644 --- a/packages/react-table/src/deprecated/components/Table/examples/Table.md +++ b/packages/react-table/src/deprecated/components/Table/examples/Table.md @@ -325,9 +325,10 @@ import { applyCellEdits, EditableTextCell, EditableSelectInputCell, - SelectOption } from '@patternfly/react-table'; +import { SelectOption as NewSelectOption} from '@patternfly/react-core/dist/esm/components/Select'; + class EditableRowsTable extends React.Component { constructor(props) { super(props); @@ -412,12 +413,12 @@ class EditableRowsTable extends React.Component { onSelect={this.onSelect} isOpen={props.isSelectOpen} options={props.options.map((option, index) => ( - + {option.value} - + ))} - onToggle={(event, isOpen) => { - this.onToggle(isOpen, rowIndex, cellIndex); + onToggle={(event) => { + this.onToggle(props.isSelectOpen, rowIndex, cellIndex); }} selections={props.selected} /> @@ -505,13 +506,13 @@ class EditableRowsTable extends React.Component { isOpen={props.isSelectOpen} options={props.options.map((option, index) => { return ( - + {option.value} - + ); })} - onToggle={(event, isOpen) => { - this.onToggle(isOpen, rowIndex, cellIndex); + onToggle={(event) => { + this.onToggle(props.isSelectOpen, rowIndex, cellIndex); }} selections={props.selected} /> @@ -622,12 +623,12 @@ class EditableRowsTable extends React.Component { clearSelection={this.clearSelection} isOpen={props.isSelectOpen} options={props.options.map((option, index) => ( - + {option.value} - + ))} - onToggle={(event, isOpen) => { - this.onToggle(isOpen, rowIndex, cellIndex); + onToggle={(event) => { + this.onToggle(props.isSelectOpen, rowIndex, cellIndex); }} selections={props.selected} /> @@ -738,7 +739,7 @@ class EditableRowsTable extends React.Component { this.onToggle = (isOpen, rowIndex, cellIndex) => { console.log('isOpen', isOpen); let newRows = Array.from(this.state.rows); - newRows[rowIndex].cells[cellIndex].props.isSelectOpen = isOpen; + newRows[rowIndex].cells[cellIndex].props.isSelectOpen = !newRows[rowIndex].cells[cellIndex].props.isSelectOpen; this.setState({ rows: newRows }); From 7225eede3f6600edf61135d78c858f8868ce9f34 Mon Sep 17 00:00:00 2001 From: Titani Date: Fri, 9 Aug 2024 14:12:05 -0400 Subject: [PATCH 2/9] more updates --- .../cypress/integration/form.spec.ts | 8 +- .../components/demos/FormDemo/FormDemo.tsx | 286 +++++++++++++++--- .../demos/TableDemo/TableEditableDemo.tsx | 15 +- .../Table/EditableSelectInputCell.tsx | 20 +- .../components/Table/examples/Table.md | 23 +- 5 files changed, 284 insertions(+), 68 deletions(-) diff --git a/packages/react-integration/cypress/integration/form.spec.ts b/packages/react-integration/cypress/integration/form.spec.ts index b8b6a684aa3..163c4f1a94b 100644 --- a/packages/react-integration/cypress/integration/form.spec.ts +++ b/packages/react-integration/cypress/integration/form.spec.ts @@ -1,4 +1,4 @@ -xdescribe('Form Demo Test', () => { +describe('Form Demo Test', () => { it('Navigate to demo section', () => { cy.visit('http://localhost:3000/form-demo-nav-link'); }); @@ -70,14 +70,14 @@ xdescribe('Form Demo Test', () => { }); it('Verify keypress can control the multi-select-typeahead', () => { - cy.get('[id*="pf-select-toggle-id"][id*="select-multi-typeahead-typeahead"]').type('{downarrow}{downarrow}{enter}'); + cy.get('.pf-v6-c-menu-toggle').type('{downarrow}{downarrow}{enter}'); cy.get('.pf-v6-c-label__text').should('exist').and('have.text', 'Florida'); cy.get('.pf-v6-c-label button').click(); - cy.get('[id*="pf-select-toggle-id"][id*="select-multi-typeahead-typeahead"]').type('New J'); + cy.get('.pf-v6-c-text-input-group__text-input').click().type('New J'); - cy.get('.pf-v6-c-select__menu-item').should('exist').and('have.text', 'New Jersey'); + cy.get('.pf-v6-c-text-input-group__text-input').should('exist').and('have.text', 'New Jersey'); cy.focused().type('{backspace}{backspace}{backspace}{backspace}orth'); cy.get('.pf-v6-c-select__menu-item').should('exist').and('have.text', 'North Carolina'); cy.focused().type('{backspace}{backspace}{backspace}{backspace}{backspace}'); diff --git a/packages/react-integration/demo-app-ts/src/components/demos/FormDemo/FormDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/FormDemo/FormDemo.tsx index c23e1009574..8089a54976a 100644 --- a/packages/react-integration/demo-app-ts/src/components/demos/FormDemo/FormDemo.tsx +++ b/packages/react-integration/demo-app-ts/src/components/demos/FormDemo/FormDemo.tsx @@ -1,12 +1,12 @@ import React, { Component } from 'react'; import { + Button, Divider, Form, FormGroup, FormGroupLabelHelp, FormProps, FormSection, - TextInput, Checkbox, Popover, ValidatedOptions, @@ -16,19 +16,42 @@ import { Select, SelectList, SelectOption, - MenuToggle + SelectOptionProps, + TextInput, + TextInputGroup, + TextInputGroupUtilities, + TextInputGroupMain, + Label, + LabelGroup, + MenuToggle, + MenuToggleElement } from '@patternfly/react-core'; import ExclamationCircleIcon from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon'; +import TimesIcon from '@patternfly/react-icons/dist/esm/icons/times-icon'; import spacing from '@patternfly/react-styles/css/utilities/Spacing/spacing'; +const initialSelectOptions = [ + { value: 'Alabama', children: 'Alabama' }, + { value: 'Florida', children: 'Florida' }, + { value: 'New Jersey', children: 'New Jersey' }, + { value: 'New Mexico', children: 'New Mexico' }, + { value: 'New York', children: 'New York' }, + { value: 'North Carolina', children: 'North Carolina' } +]; + +const NO_RESULTS = 'no results'; export interface FormState { value: string; isValid: boolean; isOpen: boolean; selected: string[]; + selectOptions: SelectOptionProps[]; validatedValue: string; validated: ValidatedOptions.default | ValidatedOptions.error | ValidatedOptions.warning | ValidatedOptions.success; checkboxChecked: boolean; + inputValue: string; + focusedItemIndex: number | null; + activeItemId: string | null; } export class FormDemo extends Component { @@ -39,19 +62,25 @@ export class FormDemo extends Component { isValid: false, isOpen: false, selected: [], + selectOptions: initialSelectOptions, validatedValue: '', validated: ValidatedOptions.default, - checkboxChecked: false + checkboxChecked: false, + inputValue: '', + focusedItemIndex: null, + activeItemId: null }; this.handleCheckboxChange = this.handleCheckboxChange.bind(this); } labelHelpRef: React.RefObject = React.createRef(); + textInputRef: React.useRef = React.createRef(); handleTextInputChange = (_event: React.FormEvent, value: string) => { this.setState({ value, isValid: /^\d+$/.test(value) }); }; + handleValidatedTextInputChange = (_event: React.FormEvent, value: string) => { let validated = ValidatedOptions.default; if (value.length === 0) { @@ -61,11 +90,28 @@ export class FormDemo extends Component { } this.setState({ validatedValue: value, validated }); }; - onToggle = (_event: any) => { + + createItemId = (value: any) => `select-multi-typeahead-${value.replace(' ', '-')}`; + + setActiveAndFocusedItem = (itemIndex: number) => { this.setState({ - isOpen: !this.state.isOpen + focusedItemIndex: itemIndex + }); + const focusedItem = this.state.selectOptions[itemIndex]; + this.setState({ + activeItemId: this.createItemId(focusedItem.value) + }); + }; + + resetActiveAndFocusedItem = () => { + this.setState({ + focusedItemIndex: null + }); + this.setState({ + activeItemId: null }); }; + onSelect = (_event: React.MouseEvent | undefined, selection: string | number | undefined) => { const { selected } = this.state; if (selection) { @@ -85,6 +131,115 @@ export class FormDemo extends Component { } }; + handleMenuArrowKeys = (key: string) => { + const { selectOptions, isOpen, focusedItemIndex } = this.state; + let indexToFocus = 0; + + if (!isOpen) { + this.setState({ + isOpen: true + }); + } + + if (selectOptions.every((option) => option.isDisabled)) { + return; + } + + if (key === 'ArrowUp') { + // When no index is set or at the first index, focus to the last, otherwise decrement focus index + if (focusedItemIndex === null || focusedItemIndex === 0) { + indexToFocus = selectOptions.length - 1; + } else { + indexToFocus = focusedItemIndex - 1; + } + + // Skip disabled options + while (selectOptions[indexToFocus].isDisabled) { + indexToFocus--; + if (indexToFocus === -1) { + indexToFocus = selectOptions.length - 1; + } + } + } + + if (key === 'ArrowDown') { + // When no index is set or at the last index, focus to the first, otherwise increment focus index + if (focusedItemIndex === null || focusedItemIndex === selectOptions.length - 1) { + indexToFocus = 0; + } else { + indexToFocus = focusedItemIndex + 1; + } + + // Skip disabled options + while (selectOptions[indexToFocus].isDisabled) { + indexToFocus++; + if (indexToFocus === selectOptions.length) { + indexToFocus = 0; + } + } + } + + this.setActiveAndFocusedItem(indexToFocus); + }; + + onInputKeyDown = (event: React.KeyboardEvent) => { + const { selectOptions, isOpen, focusedItemIndex } = this.state; + const focusedItem = focusedItemIndex !== null ? selectOptions[focusedItemIndex] : null; + + switch (event.key) { + case 'Enter': + if (isOpen && focusedItem && focusedItem.value !== NO_RESULTS && !focusedItem.isAriaDisabled) { + this.onSelect(event, focusedItem.value); + } + + if (!isOpen) { + this.setState({ + isOpen: true + }); + } + + break; + case 'ArrowUp': + case 'ArrowDown': + event.preventDefault(); + this.handleMenuArrowKeys(event.key); + break; + } + }; + + onToggle = (_event: any) => { + this.setState({ + isOpen: !this.state.isOpen + }); + this.textInputRef?.current?.focus(); + }; + + closeMenu = () => { + this.setState({ + isOpen: !this.state.isOpen + }); + this.resetActiveAndFocusedItem(); + }; + + onInputClick = () => { + if (!this.state.isOpen) { + this.setState({ isOpen: true }); + } else if (!this.state.inputValue) { + this.closeMenu(); + } + }; + + onTextInputChange = (_event: React.FormEvent, value: string) => { + this.setState({ inputValue: value }); + this.resetActiveAndFocusedItem(); + }; + onClearButtonClick = () => { + this.setState({ selected: [] }); + this.setState({ inputValue: '' }); + this.resetActiveAndFocusedItem(); + this.textInputRef?.current?.focus(); + }; + componentDidMount() { window.scrollTo(0, 0); } @@ -96,16 +251,70 @@ export class FormDemo extends Component { } render() { - const { value, isValid, isOpen, selected, validatedValue, validated, checkboxChecked } = this.state; + const { + value, + isValid, + isOpen, + selectOptions, + selected, + validatedValue, + validated, + checkboxChecked, + inputValue, + activeItemId + } = this.state; const titleId = 'multi-typeahead-select-id'; - const options = [ - { value: 'Alabama', disabled: false }, - { value: 'Florida', disabled: false }, - { value: 'New Jersey', disabled: false }, - { value: 'New Mexico', disabled: false }, - { value: 'New York', disabled: false }, - { value: 'North Carolina', disabled: false } - ]; + + const toggle = (toggleRef: React.Ref) => ( + + + + + {selected.map((selection, index) => ( + + ))} + + + +