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

feat: arrow navigation in context menu #27877

Merged
merged 19 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
cd7278f
feat: arrow navigation in context menu
samh-nl Sep 20, 2023
977d4f1
Merge branch 'main' into fix/issue-23280
samh-nl Sep 20, 2023
fa0b8e7
fix: use internal press handler on Enter
samh-nl Sep 20, 2023
5869c6c
Update src/pages/home/report/ContextMenu/BaseReportActionContextMenu.js
samh-nl Sep 21, 2023
e8d9442
Update src/pages/home/report/ContextMenu/BaseReportActionContextMenu.js
samh-nl Sep 21, 2023
20a5415
Merge branch 'main' into fix/issue-23280
samh-nl Sep 21, 2023
18a9e81
Revert "Update src/pages/home/report/ContextMenu/BaseReportActionCont…
samh-nl Sep 21, 2023
777ca93
Revert "Update src/pages/home/report/ContextMenu/BaseReportActionCont…
samh-nl Sep 21, 2023
0867e44
Revert "fix: use internal press handler on Enter"
samh-nl Sep 21, 2023
fe46818
Revert "feat: arrow navigation in context menu"
samh-nl Sep 21, 2023
85ceb1e
Revert "Revert "feat: arrow navigation in context menu""
samh-nl Sep 21, 2023
32ac964
Revert "Revert "fix: use internal press handler on Enter""
samh-nl Sep 21, 2023
56d0a92
Revert "Revert "Update src/pages/home/report/ContextMenu/BaseReportAc…
samh-nl Sep 21, 2023
c70a374
Revert "Revert "Update src/pages/home/report/ContextMenu/BaseReportAc…
samh-nl Sep 21, 2023
10bb500
fix: prevent propagation of enter
samh-nl Sep 21, 2023
6428b28
fix: stopPropagation if item is selected
samh-nl Sep 21, 2023
4d4fff3
fix: check event is set
samh-nl Sep 21, 2023
82ed8fc
Update src/components/ContextMenuItem.js
samh-nl Sep 22, 2023
ea487db
style: isFocused
samh-nl Sep 22, 2023
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
23 changes: 20 additions & 3 deletions src/components/ContextMenuItem.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, {forwardRef, useImperativeHandle} from 'react';
import PropTypes from 'prop-types';
import MenuItem from './MenuItem';
import Icon from './Icon';
Expand Down Expand Up @@ -34,6 +34,12 @@ const propTypes = {

/** The action accept for anonymous user or not */
isAnonymousAction: PropTypes.bool,

/** Whether the menu item is focused or not */
focused: PropTypes.bool,

samh-nl marked this conversation as resolved.
Show resolved Hide resolved
/** Forwarded ref to ContextMenuItem */
innerRef: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),
};

const defaultProps = {
Expand All @@ -42,9 +48,11 @@ const defaultProps = {
successText: '',
description: '',
isAnonymousAction: false,
focused: false,
innerRef: null,
};

function ContextMenuItem({onPress, successIcon, successText, icon, text, isMini, description, isAnonymousAction}) {
function ContextMenuItem({onPress, successIcon, successText, icon, text, isMini, description, isAnonymousAction, focused, innerRef}) {
const {windowWidth} = useWindowDimensions();
const [isThrottledButtonActive, setThrottledButtonInactive] = useThrottledButtonState();

Expand All @@ -61,6 +69,8 @@ function ContextMenuItem({onPress, successIcon, successText, icon, text, isMini,
}
};

useImperativeHandle(innerRef, () => ({triggerPressAndUpdateSuccess}));

const itemIcon = !isThrottledButtonActive && successIcon ? successIcon : icon;
const itemText = !isThrottledButtonActive && successText ? successText : text;

Expand Down Expand Up @@ -89,6 +99,7 @@ function ContextMenuItem({onPress, successIcon, successText, icon, text, isMini,
descriptionTextStyle={styles.breakAll}
style={getContextMenuItemStyles(windowWidth)}
isAnonymousAction={isAnonymousAction}
focused={focused}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I also thought isFocused name is better but didn't request change because focused prop name already exists here 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have adjusted the prop name in the added code.
If you'd like me to refactor the existing prop in MenuItem and change it there + it's usages also, let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's fine as we didn't touch MenuItem component here.
If anyone is adding/changing props on that component in the future, they might be responsible.
cc: @francoisl

);
}
Expand All @@ -97,4 +108,10 @@ ContextMenuItem.propTypes = propTypes;
ContextMenuItem.defaultProps = defaultProps;
ContextMenuItem.displayName = 'ContextMenuItem';

export default ContextMenuItem;
export default forwardRef((props, ref) => (
<ContextMenuItem
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
innerRef={ref}
/>
));
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useMemo, useState, memo} from 'react';
import React, {useRef, useMemo, useState, memo} from 'react';
import {InteractionManager, View} from 'react-native';
import lodashGet from 'lodash/get';
import _ from 'underscore';
Expand All @@ -15,6 +15,9 @@ import {withBetas} from '../../../../components/OnyxProvider';
import * as Session from '../../../../libs/actions/Session';
import {hideContextMenu} from './ReportActionContextMenu';
import ONYXKEYS from '../../../../ONYXKEYS';
import CONST from '../../../../CONST';
import useArrowKeyFocusManager from '../../../../hooks/useArrowKeyFocusManager';
import useKeyboardShortcut from '../../../../hooks/useKeyboardShortcut';

const propTypes = {
/** String representing the context menu type [LINK, REPORT_ACTION] which controls context menu choices */
Expand Down Expand Up @@ -45,6 +48,7 @@ const defaultProps = {
...GenericReportActionContextMenuDefaultProps,
};
function BaseReportActionContextMenu(props) {
const menuItemRefs = useRef([]);
const [shouldKeepOpen, setShouldKeepOpen] = useState(false);
samh-nl marked this conversation as resolved.
Show resolved Hide resolved
const wrapperStyle = getReportActionContextMenuStyles(props.isMini, props.isSmallScreenWidth);

Expand All @@ -58,6 +62,20 @@ function BaseReportActionContextMenu(props) {
const shouldShowFilter = (contextAction) =>
contextAction.shouldShow(props.type, reportAction, props.isArchivedRoom, props.betas, props.anchor, props.isChronosReport, props.reportID, props.isPinnedChat, props.isUnreadChat);

const shouldEnableArrowNavigation = !props.isMini && (props.isVisible || shouldKeepOpen);
const filteredContextMenuActions = _.filter(ContextMenuActions, shouldShowFilter);

// Context menu actions that are not rendered as menu items are excluded from arrow navigation
const nonMenuItemActionIndexes = _.map(filteredContextMenuActions, (contextAction, index) => (_.isFunction(contextAction.renderContent) ? index : undefined));
const disabledIndexes = _.filter(nonMenuItemActionIndexes, (index) => !_.isUndefined(index));

const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({
initialFocusedIndex: -1,
disabledIndexes,
maxIndex: filteredContextMenuActions.length - 1,
isActive: shouldEnableArrowNavigation,
});

/**
* Checks if user is anonymous. If true and the action doesn't accept for anonymous user, hides the context menu and
* shows the sign in modal. Else, executes the callback.
Expand All @@ -77,13 +95,25 @@ function BaseReportActionContextMenu(props) {
}
};

useKeyboardShortcut(
CONST.KEYBOARD_SHORTCUTS.ENTER,
() => {
if (!menuItemRefs.current[focusedIndex]) {
return;
}
menuItemRefs.current[focusedIndex].triggerPressAndUpdateSuccess();
setFocusedIndex(-1);
},
{isActive: shouldEnableArrowNavigation},
);

return (
(props.isVisible || shouldKeepOpen) && (
<View
ref={props.contentRef}
style={wrapperStyle}
>
{_.map(_.filter(ContextMenuActions, shouldShowFilter), (contextAction) => {
{_.map(_.filter(ContextMenuActions, shouldShowFilter), (contextAction, index) => {
const closePopup = !props.isMini;
samh-nl marked this conversation as resolved.
Show resolved Hide resolved
const payload = {
reportAction,
Expand All @@ -106,6 +136,9 @@ function BaseReportActionContextMenu(props) {

return (
<ContextMenuItem
ref={(ref) => {
menuItemRefs.current[index] = ref;
}}
icon={contextAction.icon}
text={props.translate(contextAction.textTranslateKey, {action: reportAction})}
successIcon={contextAction.successIcon}
Expand All @@ -115,6 +148,7 @@ function BaseReportActionContextMenu(props) {
onPress={() => interceptAnonymousUser(() => contextAction.onPress(closePopup, payload), contextAction.isAnonymousAction)}
description={contextAction.getDescription(props.selection, props.isSmallScreenWidth)}
isAnonymousAction={contextAction.isAnonymousAction}
focused={focusedIndex === index}
/>
);
})}
Expand Down
Loading