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

[MA-7]: Refactored the post priority menu and fixed the keyboard navigation issue in the menu. #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[MA-7]: Refactored the post priority menu and fixed the keyboard navigation issue in the menu. #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7bfb50
[MA-7]: Refactored the post priority menu and fixed the keyboard navi…
ayush-chauhan233 Dec 3, 2024
4f43a4f
[MA-7]: Updated the menu list structure and fixed menu not closing bug
ayush-chauhan233 Dec 9, 2024
76e6b7f
[MA-7]: Review fixes minor code structure fixes
ayush-chauhan233 Dec 11, 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
6 changes: 3 additions & 3 deletions webapp/channels/src/components/advanced_text_editor/use_priority.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {getUser} from 'mattermost-redux/selectors/entities/users';
import {openModal} from 'actions/views/modals';

import PersistNotificationConfirmModal from 'components/persist_notification_confirm_modal';
import PostPriorityPickerOverlay from 'components/post_priority/post_priority_picker_overlay';
import PostPriorityPicker from 'components/post_priority/post_priority_picker';

import Constants, {ModalIdentifiers} from 'utils/constants';
import {hasRequestedPersistentNotifications, mentionsMinusSpecialMentionsInText, specialMentionsInText} from 'utils/post_utils';
Expand All @@ -27,7 +27,7 @@ import PriorityLabels from './priority_labels';

const usePriority = (
draft: PostDraft,
handleDraftChange: ((draft: PostDraft, options: {instant?: boolean; show?: boolean}) => void),
handleDraftChange: ((draft: PostDraft, options: { instant?: boolean; show?: boolean }) => void),
focusTextbox: (keepFocus?: boolean) => void,
shouldShowPreview: boolean,
) => {
Expand Down Expand Up @@ -150,7 +150,7 @@ const usePriority = (

const additionalControl = useMemo(() =>
!rootId && isPostPriorityEnabled && (
<PostPriorityPickerOverlay
<PostPriorityPicker
key='post-priority-picker-key'
settings={draft.metadata?.priority}
onApply={handlePostPriorityApply}
Expand Down
46 changes: 28 additions & 18 deletions webapp/channels/src/components/menu/menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import Constants, {A11yClassNames} from 'utils/constants';
import {isKeyPressed} from 'utils/keyboard';

import {MenuContext, useMenuContextValue} from './menu_context';
import {MuiMenuStyled} from './menu_styled';
import {MuiPopoverStyled} from './menu_styled';

const MENU_OPEN_ANIMATION_DURATION = 150;
const MENU_CLOSE_ANIMATION_DURATION = 100;
Expand Down Expand Up @@ -60,6 +60,7 @@ type MenuProps = {
onToggle?: (isOpen: boolean) => void;
onKeyDown?: (event: KeyboardEvent<HTMLDivElement>, forceCloseMenu?: () => void) => void;
width?: string;
isMenuOpen?: boolean;
}

const defaultAnchorOrigin = {vertical: 'bottom', horizontal: 'left'};
Expand All @@ -71,6 +72,8 @@ type HorizontalOrigin = 'left' | 'center' | 'right';
interface Props {
menuButton: MenuButtonProps;
menuButtonTooltip?: MenuButtonTooltipProps;
menuHeader?: ReactNode;
menuFooter?: ReactNode;
menu: MenuProps;
children: ReactNode[];

Expand Down Expand Up @@ -105,20 +108,17 @@ export function Menu(props: Props) {
const dispatch = useDispatch();

const [anchorElement, setAnchorElement] = useState<null | HTMLElement>(null);
const [disableAutoFocusItem, setDisableAutoFocusItem] = useState(false);
const isMenuOpen = Boolean(anchorElement);

// Callback funtion handler called when menu is closed by escapeKeyDown, backdropClick or tabKeyDown
function handleMenuClose(event: MouseEvent<HTMLDivElement>) {
event.preventDefault();
setAnchorElement(null);
setDisableAutoFocusItem(false);
}

// Handle function injected into menu items to close the menu
const closeMenu = useCallback(() => {
setAnchorElement(null);
setDisableAutoFocusItem(false);
}, []);

function handleMenuModalClose(modalId: MenuProps['id']) {
Expand Down Expand Up @@ -169,6 +169,8 @@ export function Menu(props: Props) {
onModalClose: handleMenuModalClose,
children: props.children,
onKeyDown: props.menu.onKeyDown,
menuHeader: props.menuHeader,
menuFooter: props.menuFooter,
},
}),
);
Expand All @@ -177,11 +179,6 @@ export function Menu(props: Props) {
}
}

// Function to prevent focus-visible from being set on clicking menu items with the mouse
function handleMenuButtonMouseDown() {
setDisableAutoFocusItem(true);
}

// We construct the menu button so we can set onClick correctly here to support both web and mobile view
function renderMenuButton() {
const MenuButtonComponent = props.menuButton?.as ?? 'button';
Expand All @@ -197,7 +194,6 @@ export function Menu(props: Props) {
aria-label={props.menuButton?.['aria-label'] ?? ''}
className={props.menuButton?.class ?? ''}
onClick={handleMenuButtonClick}
onMouseDown={handleMenuButtonMouseDown}
>
{props.menuButton.children}
</MenuButtonComponent>
Expand Down Expand Up @@ -225,6 +221,12 @@ export function Menu(props: Props) {
}
}, [isMenuOpen]);

useEffect(() => {
if (props.menu.isMenuOpen === false) {
setAnchorElement(null);
}
}, [props.menu.isMenuOpen]);

const providerValue = useMenuContextValue(closeMenu, Boolean(anchorElement));

if (isMobileView) {
Expand All @@ -236,7 +238,8 @@ export function Menu(props: Props) {
<CompassDesignProvider theme={theme}>
{renderMenuButton()}
<MenuContext.Provider value={providerValue}>
<MuiMenuStyled
<MuiPopoverStyled
aria-label={props.menu?.['aria-label']}
anchorEl={anchorElement}
open={isMenuOpen}
onClose={handleMenuClose}
Expand All @@ -247,11 +250,6 @@ export function Menu(props: Props) {
width={props.menu.width}
anchorOrigin={props.anchorOrigin || defaultAnchorOrigin}
transformOrigin={props.transformOrigin || defaultTransformOrigin}
disableAutoFocusItem={disableAutoFocusItem} // This is not anti-pattern, see handleMenuButtonMouseDown
MenuListProps={{
id: props.menu.id,
'aria-label': props.menu?.['aria-label'] ?? '',
}}
TransitionProps={{
mountOnEnter: true,
unmountOnExit: true,
Expand All @@ -261,8 +259,16 @@ export function Menu(props: Props) {
},
}}
>
{props.children}
</MuiMenuStyled>
{props.menuHeader}
<MuiMenuList
id={props.menu.id}
aria-label={props.menu?.['aria-label']}
autoFocusItem={true}
>
{props.children}
</MuiMenuList>
{props.menuFooter}
</MuiPopoverStyled>
</MenuContext.Provider>
</CompassDesignProvider>
);
Expand All @@ -275,6 +281,8 @@ interface MenuModalProps {
onModalClose: (modalId: MenuProps['id']) => void;
children: Props['children'];
onKeyDown?: MenuProps['onKeyDown'];
menuHeader?: Props['menuHeader'];
menuFooter?: Props['menuFooter'];
}

function MenuModal(props: MenuModalProps) {
Expand Down Expand Up @@ -319,7 +327,9 @@ function MenuModal(props: MenuModalProps) {
aria-labelledby={props.menuButtonId}
onClick={handleModalClickCapture}
>
{props.menuHeader}
{props.children}
{props.menuFooter}
</MuiMenuList>
</GenericModal>
</CompassDesignProvider>
Expand Down
1 change: 0 additions & 1 deletion webapp/channels/src/components/menu/menu_item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ export const MenuItemStyled = styled(MuiMenuItem, {
justifyContent: 'flex-start',
alignItems: hasOnlyPrimaryLabel || isLabelsRowLayout ? 'center' : 'flex-start',
minHeight: '36px',
maxHeight: '56px',

// aria expanded to add the active styling on parent sub menu item
'&.Mui-active, &[aria-expanded="true"]': {
Expand Down
6 changes: 4 additions & 2 deletions webapp/channels/src/components/menu/menu_item_separator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// See LICENSE.txt for license information.

import {Divider} from '@mui/material';
import type {DividerProps} from '@mui/material';
import type {ElementType} from 'react';
import React from 'react';

/**
Expand All @@ -12,11 +14,11 @@ import React from 'react';
* <Menu.Separator />
* </Menu.Container>
*/
export function MenuItemSeparator() {
export function MenuItemSeparator(props: DividerProps & {component?: ElementType }) {
return (
<Divider
component='li'
aria-orientation='vertical'
{...props}
/>
);
}
31 changes: 29 additions & 2 deletions webapp/channels/src/components/menu/menu_styled.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,16 @@

import MuiMenu from '@mui/material/Menu';
import type {MenuProps as MuiMenuProps} from '@mui/material/Menu';
import Popover from '@mui/material/Popover';
import type {PopoverProps as MuiPopoverProps} from '@mui/material/Popover';
import {styled} from '@mui/material/styles';

interface Props extends MuiMenuProps {
interface PopoverProps extends MuiPopoverProps {
asSubMenu?: boolean;
width?: string;
}

interface MenuProps extends MuiMenuProps {
asSubMenu?: boolean;
width?: string;
}
Expand All @@ -16,7 +23,27 @@ interface Props extends MuiMenuProps {
*/
export const MuiMenuStyled = styled(MuiMenu, {
shouldForwardProp: (prop) => prop !== 'asSubMenu',
})<Props>(
})<MenuProps>(
({asSubMenu, width}) => ({
'& .MuiPaper-root': {
backgroundColor: 'var(--center-channel-bg)',
boxShadow: `${asSubMenu ? 'var(--elevation-5)' : 'var(--elevation-4)'
}, 0 0 0 1px rgba(var(--center-channel-color-rgb), 0.12) inset`,
minWidth: '114px',
maxWidth: '496px',
maxHeight: '80vh',
width,
},
}),
);

/**
* A styled version of the Material-UI Popover component with few overrides.
* @warning This component is meant to be only used inside of the Menu component directory.
*/
export const MuiPopoverStyled = styled(Popover, {
shouldForwardProp: (prop) => prop !== 'asSubMenu',
})<PopoverProps>(
({asSubMenu, width}) => ({
'& .MuiPaper-root': {
backgroundColor: 'var(--center-channel-bg)',
Expand Down
Loading
Loading