From 684098594e8572af384c1a87f43e0890f8227874 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 4 Dec 2023 15:08:50 +0700 Subject: [PATCH 1/7] prevent enter key subscribe --- .../MoneyRequestConfirmationList.js | 2 +- .../OptionsSelector/BaseOptionsSelector.js | 39 ++++++++++++++++++- .../Pressable/GenericPressable/index.tsx | 2 +- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/components/MoneyRequestConfirmationList.js b/src/components/MoneyRequestConfirmationList.js index 1b4967a9c54c..1c22c2452da8 100755 --- a/src/components/MoneyRequestConfirmationList.js +++ b/src/components/MoneyRequestConfirmationList.js @@ -520,7 +520,7 @@ function MoneyRequestConfirmationList(props) { /> ) : ( confirm(value)} options={splitOrRequestOptions} diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index 9c8b76515fe8..48683e4e5dbe 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -80,11 +80,15 @@ class BaseOptionsSelector extends Component { this.incrementPage = this.incrementPage.bind(this); this.sliceSections = this.sliceSections.bind(this); this.calculateAllVisibleOptionsCount = this.calculateAllVisibleOptionsCount.bind(this); + this.handleFocusIn = this.handleFocusIn.bind(this); + this.handleFocusOut = this.handleFocusOut.bind(this); this.relatedTarget = null; const allOptions = this.flattenSections(); const sections = this.sliceSections(); const focusedIndex = this.getInitiallyFocusedIndex(allOptions); + const platform = getPlatform(); + const isNative = platform === CONST.PLATFORM.IOS || platform === CONST.PLATFORM.ANDROID; this.state = { sections, @@ -94,11 +98,13 @@ class BaseOptionsSelector extends Component { shouldShowReferralModal: false, errorMessage: '', paginationPage: 1, + disableEnterShortCut: isNative ? false : this.handleFocusIn(), }; } componentDidMount() { this.subscribeToKeyboardShortcut(); + this.subscribeActiveElement(); if (this.props.isFocused && this.props.autoFocus && this.textInput) { this.focusTimeout = setTimeout(() => { @@ -118,6 +124,11 @@ class BaseOptionsSelector extends Component { } } + if (prevState.disableEnterShortCut !== this.state.disableEnterShortCut) { + this.unSubscribeFromKeyboardShortcut(); + this.subscribeToKeyboardShortcut(); + } + // Screen coming back into focus, for example // when doing Cmd+Shift+K, then Cmd+K, then Cmd+Shift+K. // Only applies to platforms that support keyboard shortcuts @@ -181,6 +192,7 @@ class BaseOptionsSelector extends Component { } this.unSubscribeFromKeyboardShortcut(); + this.unSubscribeActiveElement(); } /** @@ -256,11 +268,36 @@ class BaseOptionsSelector extends Component { this.setState((prevState) => ({shouldShowReferralModal: !prevState.shouldShowReferralModal})); } + handleFocusIn() { + const activeElement = document.activeElement; + console.log(activeElement.role); + this.setState({ + disableEnterShortCut: activeElement && [CONST.ACCESSIBILITY_ROLE.BUTTON, CONST.ACCESSIBILITY_ROLE.CHECKBOX].includes(activeElement.role) + + }) + } + + handleFocusOut() { + this.setState({ + disableEnterShortCut: false + }) + } + + subscribeActiveElement() { + document.addEventListener('focusin', this.handleFocusIn); + document.addEventListener('focusout', this.handleFocusOut); + } + + unSubscribeActiveElement() { + document.removeEventListener('focusin', this.handleFocusIn); + document.removeEventListener('focusout', this.handleFocusOut); + } + subscribeToKeyboardShortcut() { const enterConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; this.unsubscribeEnter = KeyboardShortcut.subscribe( enterConfig.shortcutKey, - this.selectFocusedOption, + !this.state.disableEnterShortCut ? this.selectFocusedOption: undefined, enterConfig.descriptionKey, enterConfig.modifiers, true, diff --git a/src/components/Pressable/GenericPressable/index.tsx b/src/components/Pressable/GenericPressable/index.tsx index e050e23c8e9a..94275bc234c5 100644 --- a/src/components/Pressable/GenericPressable/index.tsx +++ b/src/components/Pressable/GenericPressable/index.tsx @@ -14,7 +14,7 @@ function WebGenericPressable({focusable = true, ...props}: PressableProps, ref: // change native accessibility props to web accessibility props focusable={focusable} tabIndex={props.tabIndex ?? (!accessible || !focusable) ? -1 : 0} - role={props.accessibilityRole as Role} + role={(props.accessibilityRole || props.role) as Role} id={props.nativeID} aria-label={props.accessibilityLabel} aria-labelledby={props.accessibilityLabelledBy} From 31109b8f84f61cd0e1a81ac97e408016a847d2a2 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 4 Dec 2023 16:16:31 +0700 Subject: [PATCH 2/7] check checkbox only --- src/components/MoneyRequestConfirmationList.js | 2 +- src/components/OptionsSelector/BaseOptionsSelector.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/MoneyRequestConfirmationList.js b/src/components/MoneyRequestConfirmationList.js index 1c22c2452da8..1b4967a9c54c 100755 --- a/src/components/MoneyRequestConfirmationList.js +++ b/src/components/MoneyRequestConfirmationList.js @@ -520,7 +520,7 @@ function MoneyRequestConfirmationList(props) { /> ) : ( confirm(value)} options={splitOrRequestOptions} diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index 48683e4e5dbe..b4ba98b5b39f 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -270,9 +270,8 @@ class BaseOptionsSelector extends Component { handleFocusIn() { const activeElement = document.activeElement; - console.log(activeElement.role); this.setState({ - disableEnterShortCut: activeElement && [CONST.ACCESSIBILITY_ROLE.BUTTON, CONST.ACCESSIBILITY_ROLE.CHECKBOX].includes(activeElement.role) + disableEnterShortCut: activeElement && [CONST.ACCESSIBILITY_ROLE.CHECKBOX].includes(activeElement.role) }) } From 721729f57e2d53094576179b6dd51741ba8401b7 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Tue, 5 Dec 2023 10:58:04 +0700 Subject: [PATCH 3/7] fix lint --- src/components/OptionsSelector/BaseOptionsSelector.js | 11 +++++------ src/components/Pressable/GenericPressable/index.tsx | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index b4ba98b5b39f..b3e338b601ab 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -271,15 +271,14 @@ class BaseOptionsSelector extends Component { handleFocusIn() { const activeElement = document.activeElement; this.setState({ - disableEnterShortCut: activeElement && [CONST.ACCESSIBILITY_ROLE.CHECKBOX].includes(activeElement.role) - - }) + disableEnterShortCut: activeElement && [CONST.ACCESSIBILITY_ROLE.CHECKBOX].includes(activeElement.role), + }); } handleFocusOut() { this.setState({ - disableEnterShortCut: false - }) + disableEnterShortCut: false, + }); } subscribeActiveElement() { @@ -296,7 +295,7 @@ class BaseOptionsSelector extends Component { const enterConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; this.unsubscribeEnter = KeyboardShortcut.subscribe( enterConfig.shortcutKey, - !this.state.disableEnterShortCut ? this.selectFocusedOption: undefined, + !this.state.disableEnterShortCut ? this.selectFocusedOption : undefined, enterConfig.descriptionKey, enterConfig.modifiers, true, diff --git a/src/components/Pressable/GenericPressable/index.tsx b/src/components/Pressable/GenericPressable/index.tsx index 94275bc234c5..90bad15756fb 100644 --- a/src/components/Pressable/GenericPressable/index.tsx +++ b/src/components/Pressable/GenericPressable/index.tsx @@ -14,7 +14,7 @@ function WebGenericPressable({focusable = true, ...props}: PressableProps, ref: // change native accessibility props to web accessibility props focusable={focusable} tabIndex={props.tabIndex ?? (!accessible || !focusable) ? -1 : 0} - role={(props.accessibilityRole || props.role) as Role} + role={(props.accessibilityRole ?? props.role) as Role} id={props.nativeID} aria-label={props.accessibilityLabel} aria-labelledby={props.accessibilityLabelledBy} From 3d3964a50024ec016c15dfbcb9790118936e6243 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Thu, 7 Dec 2023 13:21:42 +0700 Subject: [PATCH 4/7] fix focus for element --- .../OptionsSelector/BaseOptionsSelector.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index 4b323187cc1b..ffe044b49a7d 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -83,6 +83,7 @@ class BaseOptionsSelector extends Component { this.handleFocusIn = this.handleFocusIn.bind(this); this.handleFocusOut = this.handleFocusOut.bind(this); this.relatedTarget = null; + this.accessibilityRoles = _.values(CONST.ACCESSIBILITY_ROLE); const allOptions = this.flattenSections(); const sections = this.sliceSections(); @@ -125,8 +126,11 @@ class BaseOptionsSelector extends Component { } if (prevState.disableEnterShortCut !== this.state.disableEnterShortCut) { - this.unSubscribeFromKeyboardShortcut(); - this.subscribeToKeyboardShortcut(); + if (this.state.disableEnterShortCut) { + this.unSubscribeFromKeyboardShortcut(); + } else { + this.subscribeToKeyboardShortcut(); + } } // Screen coming back into focus, for example @@ -271,7 +275,7 @@ class BaseOptionsSelector extends Component { handleFocusIn() { const activeElement = document.activeElement; this.setState({ - disableEnterShortCut: activeElement && [CONST.ACCESSIBILITY_ROLE.CHECKBOX].includes(activeElement.role), + disableEnterShortCut: activeElement && this.accessibilityRoles.includes(activeElement.role) && activeElement.role !== CONST.ACCESSIBILITY_ROLE.TEXT, }); } @@ -282,11 +286,17 @@ class BaseOptionsSelector extends Component { } subscribeActiveElement() { + if (![CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) { + return; + } document.addEventListener('focusin', this.handleFocusIn); document.addEventListener('focusout', this.handleFocusOut); } unSubscribeActiveElement() { + if (![CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) { + return; + } document.removeEventListener('focusin', this.handleFocusIn); document.removeEventListener('focusout', this.handleFocusOut); } @@ -295,7 +305,7 @@ class BaseOptionsSelector extends Component { const enterConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; this.unsubscribeEnter = KeyboardShortcut.subscribe( enterConfig.shortcutKey, - !this.state.disableEnterShortCut ? this.selectFocusedOption : undefined, + this.selectFocusedOption, enterConfig.descriptionKey, enterConfig.modifiers, true, From 11ffb053e09556a328344fb48f3e12cd79d6f865 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Wed, 3 Jan 2024 14:25:45 +0700 Subject: [PATCH 5/7] change role of circle icon in OptionRow --- src/components/OptionRow.tsx | 4 ++-- src/components/OptionsSelector/BaseOptionsSelector.js | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/OptionRow.tsx b/src/components/OptionRow.tsx index 5a2f6902c4c0..7e4ac88ce957 100644 --- a/src/components/OptionRow.tsx +++ b/src/components/OptionRow.tsx @@ -267,8 +267,8 @@ function OptionRow({ onSelectedStatePressed(option)} disabled={isDisabled} - role={CONST.ROLE.CHECKBOX} - accessibilityLabel={CONST.ROLE.CHECKBOX} + role={CONST.ROLE.BUTTON} + accessibilityLabel={CONST.ROLE.BUTTON} > diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index fd494a997e64..6232d12dfa24 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -84,12 +84,11 @@ class BaseOptionsSelector extends Component { this.handleFocusOut = this.handleFocusOut.bind(this); this.debouncedUpdateSearchValue = _.debounce(this.updateSearchValue, CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME); this.relatedTarget = null; - this.accessibilityRoles = _.values(CONST.ACCESSIBILITY_ROLE); + this.accessibilityRoles = _.values(CONST.ROLE); const allOptions = this.flattenSections(); const sections = this.sliceSections(); const focusedIndex = this.getInitiallyFocusedIndex(allOptions); - const platform = getPlatform(); this.state = { sections, @@ -277,7 +276,7 @@ class BaseOptionsSelector extends Component { handleFocusIn() { const activeElement = document.activeElement; this.setState({ - disableEnterShortCut: activeElement && this.accessibilityRoles.includes(activeElement.role) && activeElement.role !== CONST.ACCESSIBILITY_ROLE.TEXT, + disableEnterShortCut: activeElement && this.accessibilityRoles.includes(activeElement.role) && activeElement.role !== CONST.ROLE.PRESENTATION, }); } From 0b9f6df9636fee73ec93c307e706ccbd32a00012 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Tue, 9 Jan 2024 19:41:00 +0700 Subject: [PATCH 6/7] only disable enter short cut --- .../OptionsSelector/BaseOptionsSelector.js | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/components/OptionsSelector/BaseOptionsSelector.js b/src/components/OptionsSelector/BaseOptionsSelector.js index 6232d12dfa24..196a67383b21 100755 --- a/src/components/OptionsSelector/BaseOptionsSelector.js +++ b/src/components/OptionsSelector/BaseOptionsSelector.js @@ -85,6 +85,7 @@ class BaseOptionsSelector extends Component { this.debouncedUpdateSearchValue = _.debounce(this.updateSearchValue, CONST.TIMING.SEARCH_OPTION_LIST_DEBOUNCE_TIME); this.relatedTarget = null; this.accessibilityRoles = _.values(CONST.ROLE); + this.isWebOrDesktop = [CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform()); const allOptions = this.flattenSections(); const sections = this.sliceSections(); @@ -104,7 +105,8 @@ class BaseOptionsSelector extends Component { } componentDidMount() { - this.subscribeToKeyboardShortcut(); + this.subscribeToEnterShortcut(); + this.subscribeToCtrlEnterShortcut(); this.subscribeActiveElement(); if (this.props.isFocused && this.props.autoFocus && this.textInput) { @@ -119,7 +121,8 @@ class BaseOptionsSelector extends Component { componentDidUpdate(prevProps, prevState) { if (prevProps.isFocused !== this.props.isFocused) { if (this.props.isFocused) { - this.subscribeToKeyboardShortcut(); + this.subscribeToEnterShortcut(); + this.subscribeToCtrlEnterShortcut(); } else { this.unSubscribeFromKeyboardShortcut(); } @@ -127,16 +130,16 @@ class BaseOptionsSelector extends Component { if (prevState.disableEnterShortCut !== this.state.disableEnterShortCut) { if (this.state.disableEnterShortCut) { - this.unSubscribeFromKeyboardShortcut(); + this.unsubscribeEnter(); } else { - this.subscribeToKeyboardShortcut(); + this.subscribeToEnterShortcut(); } } // Screen coming back into focus, for example // when doing Cmd+Shift+K, then Cmd+K, then Cmd+Shift+K. // Only applies to platforms that support keyboard shortcuts - if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform()) && !prevProps.isFocused && this.props.isFocused && this.props.autoFocus && this.textInput) { + if (this.isWebOrDesktop && !prevProps.isFocused && this.props.isFocused && this.props.autoFocus && this.textInput) { setTimeout(() => { this.textInput.focus(); }, CONST.ANIMATED_TRANSITION); @@ -287,7 +290,7 @@ class BaseOptionsSelector extends Component { } subscribeActiveElement() { - if (![CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) { + if (!this.isWebOrDesktop) { return; } document.addEventListener('focusin', this.handleFocusIn); @@ -295,14 +298,14 @@ class BaseOptionsSelector extends Component { } unSubscribeActiveElement() { - if (![CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) { + if (!this.isWebOrDesktop) { return; } document.removeEventListener('focusin', this.handleFocusIn); document.removeEventListener('focusout', this.handleFocusOut); } - subscribeToKeyboardShortcut() { + subscribeToEnterShortcut() { const enterConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; this.unsubscribeEnter = KeyboardShortcut.subscribe( enterConfig.shortcutKey, @@ -312,7 +315,9 @@ class BaseOptionsSelector extends Component { true, () => !this.state.allOptions[this.state.focusedIndex], ); + } + subscribeToCtrlEnterShortcut() { const CTRLEnterConfig = CONST.KEYBOARD_SHORTCUTS.CTRL_ENTER; this.unsubscribeCTRLEnter = KeyboardShortcut.subscribe( CTRLEnterConfig.shortcutKey, From db089da745834b44617184d5e8e1e7ad9796a701 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Wed, 10 Jan 2024 12:46:48 +0700 Subject: [PATCH 7/7] revert hard code --- .../MoneyTemporaryForRefactorRequestConfirmationList.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js b/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js index 860a61c429f8..2fee67a3d632 100755 --- a/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js +++ b/src/components/MoneyTemporaryForRefactorRequestConfirmationList.js @@ -571,7 +571,7 @@ function MoneyTemporaryForRefactorRequestConfirmationList({ const button = shouldShowSettlementButton ? ( ) : ( confirm(value)} options={splitOrRequestOptions}