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.

Sign up for GitHub

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

Prevent enter key event if the focused element is a clickable element #32464

Merged
merged 16 commits into from
Jan 12, 2024
Merged

Prevent enter key event if the focused element is a clickable element #32464

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6840985
prevent enter key subscribe
dukenv0307 Dec 4, 2023
31109b8
check checkbox only
dukenv0307 Dec 4, 2023
721729f
fix lint
dukenv0307 Dec 5, 2023
0fea640
Merge branch 'main' into fix/21392
dukenv0307 Dec 7, 2023
3d3964a
fix focus for element
dukenv0307 Dec 7, 2023
642ee58
merge main
dukenv0307 Dec 25, 2023
9cbbd29
Merge branch 'main' into fix/21392
dukenv0307 Jan 3, 2024
11ffb05
change role of circle icon in OptionRow
dukenv0307 Jan 3, 2024
0b9f6df
only disable enter short cut
dukenv0307 Jan 9, 2024
b1b29b7
Merge branch 'main' into fix/21392
dukenv0307 Jan 9, 2024
654e4ed
merge main
dukenv0307 Jan 10, 2024
db089da
revert hard code
dukenv0307 Jan 10, 2024
a6c1687
merge main
dukenv0307 Jan 11, 2024
3b93bf2
Merge branch 'main' of https://github.com/dukenv0307/App into fix/21392
dukenv0307 Jan 11, 2024
fe22006
Merge branch 'main' into fix/21392
dukenv0307 Jan 11, 2024
2d77f12
Merge branch 'main' into fix/21392
dukenv0307 Jan 12, 2024
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
4 changes: 2 additions & 2 deletions src/components/OptionRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ function OptionRow({
<PressableWithFeedback
onPress={() => onSelectedStatePressed(option)}
disabled={isDisabled}
role={CONST.ROLE.CHECKBOX}
accessibilityLabel={CONST.ROLE.CHECKBOX}
role={CONST.ROLE.BUTTON}
accessibilityLabel={CONST.ROLE.BUTTON}
>
<SelectCircle isChecked={isSelected} />
</PressableWithFeedback>
Expand Down
55 changes: 51 additions & 4 deletions src/components/OptionsSelector/BaseOptionsSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,14 @@ 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.onLayout = this.onLayout.bind(this);
this.setListRef = this.setListRef.bind(this);
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());

this.focusListener = null;
this.blurListener = null;
Expand All @@ -94,6 +98,7 @@ class BaseOptionsSelector extends Component {
shouldShowReferralModal: false,
errorMessage: '',
paginationPage: 1,
disableEnterShortCut: false,
value: '',
};
}
Expand All @@ -103,8 +108,10 @@ class BaseOptionsSelector extends Component {
// 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())) {
this.subscribeToKeyboardShortcut();
if (this.isWebOrDesktop) {
this.subscribeToEnterShortcut();
this.subscribeToCtrlEnterShortcut();
this.subscribeActiveElement();
}

if (this.props.autoFocus && this.textInput) {
Expand All @@ -117,8 +124,9 @@ class BaseOptionsSelector extends Component {
});

this.blurListener = this.props.navigation.addListener('blur', () => {
if ([CONST.PLATFORM.DESKTOP, CONST.PLATFORM.WEB].includes(getPlatform())) {
if (this.isWebOrDesktop) {
this.unSubscribeFromKeyboardShortcut();
this.unSubscribeActiveElement();
}
this.isFocused = false;
});
Expand All @@ -142,6 +150,14 @@ class BaseOptionsSelector extends Component {
}

componentDidUpdate(prevProps, prevState) {
if (prevState.disableEnterShortCut !== this.state.disableEnterShortCut) {
if (this.state.disableEnterShortCut) {
this.unsubscribeEnter();
} else {
this.subscribeToEnterShortcut();
Comment on lines +122 to +126
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused regression of enter key sometimes not working while navigating - #35373
More details about the root cause: #35373 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

Caused similar regression where enter key not working while saving ws name #36923

}
}

if (prevState.paginationPage !== this.state.paginationPage) {
const newSections = this.sliceSections();

Expand Down Expand Up @@ -289,7 +305,36 @@ class BaseOptionsSelector extends Component {
this.setState((prevState) => ({shouldShowReferralModal: !prevState.shouldShowReferralModal}));
}

subscribeToKeyboardShortcut() {
handleFocusIn() {
const activeElement = document.activeElement;
this.setState({
disableEnterShortCut: activeElement && this.accessibilityRoles.includes(activeElement.role) && activeElement.role !== CONST.ROLE.PRESENTATION,
});
}

handleFocusOut() {
this.setState({
disableEnterShortCut: false,
});
}

subscribeActiveElement() {
if (!this.isWebOrDesktop) {
return;
}
document.addEventListener('focusin', this.handleFocusIn);
document.addEventListener('focusout', this.handleFocusOut);
}

unSubscribeActiveElement() {
if (!this.isWebOrDesktop) {
return;
}
document.removeEventListener('focusin', this.handleFocusIn);
document.removeEventListener('focusout', this.handleFocusOut);
}

subscribeToEnterShortcut() {
const enterConfig = CONST.KEYBOARD_SHORTCUTS.ENTER;
this.unsubscribeEnter = KeyboardShortcut.subscribe(
enterConfig.shortcutKey,
Expand All @@ -299,7 +344,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,
Expand Down