Prevent useContextMenu isOpen from being true if the button ref goes away (#9418)

This commit is contained in:
Michael Telatynski 2022-10-17 17:42:04 +01:00 committed by GitHub
parent e1d631cb47
commit 13db1b17be
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 38 additions and 35 deletions

View file

@ -565,8 +565,13 @@ type ContextMenuTuple<T> = [
(val: boolean) => void, (val: boolean) => void,
]; ];
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint
export const useContextMenu = <T extends any = HTMLElement>(): ContextMenuTuple<T> => { export const useContextMenu = <T extends any = HTMLElement>(inputRef?: RefObject<T>): ContextMenuTuple<T> => {
const button = useRef<T>(null); let button = useRef<T>(null);
if (inputRef) {
// if we are given a ref, use it instead of ours
button = inputRef;
}
const [isOpen, setIsOpen] = useState(false); const [isOpen, setIsOpen] = useState(false);
const open = (ev?: SyntheticEvent) => { const open = (ev?: SyntheticEvent) => {
ev?.preventDefault(); ev?.preventDefault();
@ -579,7 +584,7 @@ export const useContextMenu = <T extends any = HTMLElement>(): ContextMenuTuple<
setIsOpen(false); setIsOpen(false);
}; };
return [isOpen, button, open, close, setIsOpen]; return [button.current ? isOpen : false, button, open, close, setIsOpen];
}; };
// XXX: Deprecated, used only for dynamic Tooltips. Avoid using at all costs. // XXX: Deprecated, used only for dynamic Tooltips. Avoid using at all costs.

View file

@ -69,6 +69,7 @@ export const LocationButton: React.FC<IProps> = ({ roomId, sender, menuPosition,
iconClassName="mx_MessageComposer_location" iconClassName="mx_MessageComposer_location"
onClick={openMenu} onClick={openMenu}
title={_t("Location")} title={_t("Location")}
inputRef={button}
/> />
{ contextMenu } { contextMenu }

View file

@ -209,7 +209,8 @@ export function ReadReceiptGroup(
onMouseOver={showTooltip} onMouseOver={showTooltip}
onMouseLeave={hideTooltip} onMouseLeave={hideTooltip}
onFocus={showTooltip} onFocus={showTooltip}
onBlur={hideTooltip}> onBlur={hideTooltip}
>
{ remText } { remText }
<span <span
className="mx_ReadReceiptGroup_container" className="mx_ReadReceiptGroup_container"

View file

@ -204,9 +204,7 @@ const CreateSpaceButton = ({
isPanelCollapsed, isPanelCollapsed,
setPanelCollapsed, setPanelCollapsed,
}: Pick<IInnerSpacePanelProps, "isPanelCollapsed" | "setPanelCollapsed">) => { }: Pick<IInnerSpacePanelProps, "isPanelCollapsed" | "setPanelCollapsed">) => {
// We don't need the handle as we position the menu in a constant location const [menuDisplayed, handle, openMenu, closeMenu] = useContextMenu<HTMLElement>();
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [menuDisplayed, _handle, openMenu, closeMenu] = useContextMenu<void>();
useEffect(() => { useEffect(() => {
if (!isPanelCollapsed && menuDisplayed) { if (!isPanelCollapsed && menuDisplayed) {
@ -231,13 +229,14 @@ const CreateSpaceButton = ({
role="treeitem" role="treeitem"
> >
<SpaceButton <SpaceButton
data-test-id='create-space-button' data-testid='create-space-button'
className={classNames("mx_SpaceButton_new", { className={classNames("mx_SpaceButton_new", {
mx_SpaceButton_newCancel: menuDisplayed, mx_SpaceButton_newCancel: menuDisplayed,
})} })}
label={menuDisplayed ? _t("Cancel") : _t("Create a space")} label={menuDisplayed ? _t("Cancel") : _t("Create a space")}
onClick={onNewClick} onClick={onNewClick}
isNarrow={isPanelCollapsed} isNarrow={isPanelCollapsed}
ref={handle}
/> />
{ contextMenu } { contextMenu }

View file

@ -14,7 +14,16 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
import React, { MouseEvent, ComponentProps, ComponentType, createRef, InputHTMLAttributes, LegacyRef } from "react"; import React, {
MouseEvent,
ComponentProps,
ComponentType,
createRef,
InputHTMLAttributes,
LegacyRef,
forwardRef,
RefObject,
} from "react";
import classNames from "classnames"; import classNames from "classnames";
import { Room, RoomEvent } from "matrix-js-sdk/src/models/room"; import { Room, RoomEvent } from "matrix-js-sdk/src/models/room";
import { DraggableProvidedDragHandleProps } from "react-beautiful-dnd"; import { DraggableProvidedDragHandleProps } from "react-beautiful-dnd";
@ -54,7 +63,7 @@ interface IButtonProps extends Omit<ComponentProps<typeof AccessibleTooltipButto
onClick?(ev?: ButtonEvent): void; onClick?(ev?: ButtonEvent): void;
} }
export const SpaceButton: React.FC<IButtonProps> = ({ export const SpaceButton = forwardRef<HTMLElement, IButtonProps>(({
space, space,
spaceKey, spaceKey,
className, className,
@ -67,9 +76,9 @@ export const SpaceButton: React.FC<IButtonProps> = ({
children, children,
ContextMenuComponent, ContextMenuComponent,
...props ...props
}) => { }, ref: RefObject<HTMLElement>) => {
const [menuDisplayed, ref, openMenu, closeMenu] = useContextMenu<HTMLElement>(); const [menuDisplayed, handle, openMenu, closeMenu] = useContextMenu<HTMLElement>(ref);
const [onFocus, isActive, handle] = useRovingTabIndex(ref); const [onFocus, isActive] = useRovingTabIndex(handle);
const tabIndex = isActive ? 0 : -1; const tabIndex = isActive ? 0 : -1;
let avatar = <div className="mx_SpaceButton_avatarPlaceholder"><div className="mx_SpaceButton_icon" /></div>; let avatar = <div className="mx_SpaceButton_avatarPlaceholder"><div className="mx_SpaceButton_icon" /></div>;
@ -150,7 +159,7 @@ export const SpaceButton: React.FC<IButtonProps> = ({
</div> </div>
</AccessibleTooltipButton> </AccessibleTooltipButton>
); );
}; });
interface IItemProps extends InputHTMLAttributes<HTMLLIElement> { interface IItemProps extends InputHTMLAttributes<HTMLLIElement> {
space: Room; space: Room;

View file

@ -15,16 +15,13 @@ limitations under the License.
*/ */
import React from 'react'; import React from 'react';
// eslint-disable-next-line deprecate/import import { render, screen, fireEvent } from "@testing-library/react";
import { mount } from 'enzyme';
import { mocked } from 'jest-mock'; import { mocked } from 'jest-mock';
import { MatrixClient } from 'matrix-js-sdk/src/matrix'; import { MatrixClient } from 'matrix-js-sdk/src/matrix';
import { act } from "react-dom/test-utils";
import SpacePanel from '../../../../src/components/views/spaces/SpacePanel'; import SpacePanel from '../../../../src/components/views/spaces/SpacePanel';
import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; import { MatrixClientPeg } from '../../../../src/MatrixClientPeg';
import { SpaceKey } from '../../../../src/stores/spaces'; import { SpaceKey } from '../../../../src/stores/spaces';
import { findByTestId } from '../../../test-utils';
import { shouldShowComponent } from '../../../../src/customisations/helpers/UIComponents'; import { shouldShowComponent } from '../../../../src/customisations/helpers/UIComponents';
import { UIComponent } from '../../../../src/settings/UIFeature'; import { UIComponent } from '../../../../src/settings/UIFeature';
@ -47,10 +44,6 @@ jest.mock('../../../../src/customisations/helpers/UIComponents', () => ({
})); }));
describe('<SpacePanel />', () => { describe('<SpacePanel />', () => {
const defaultProps = {};
const getComponent = (props = {}) =>
mount(<SpacePanel {...defaultProps} {...props} />);
const mockClient = { const mockClient = {
getUserId: jest.fn().mockReturnValue('@test:test'), getUserId: jest.fn().mockReturnValue('@test:test'),
isGuest: jest.fn(), isGuest: jest.fn(),
@ -67,26 +60,21 @@ describe('<SpacePanel />', () => {
describe('create new space button', () => { describe('create new space button', () => {
it('renders create space button when UIComponent.CreateSpaces component should be shown', () => { it('renders create space button when UIComponent.CreateSpaces component should be shown', () => {
const component = getComponent(); render(<SpacePanel />);
expect(findByTestId(component, 'create-space-button').length).toBeTruthy(); screen.getByTestId("create-space-button");
}); });
it('does not render create space button when UIComponent.CreateSpaces component should not be shown', () => { it('does not render create space button when UIComponent.CreateSpaces component should not be shown', () => {
mocked(shouldShowComponent).mockReturnValue(false); mocked(shouldShowComponent).mockReturnValue(false);
const component = getComponent(); render(<SpacePanel />);
expect(shouldShowComponent).toHaveBeenCalledWith(UIComponent.CreateSpaces); expect(shouldShowComponent).toHaveBeenCalledWith(UIComponent.CreateSpaces);
expect(findByTestId(component, 'create-space-button').length).toBeFalsy(); expect(screen.queryByTestId("create-space-button")).toBeFalsy();
}); });
it('opens context menu on create space button click', async () => { it('opens context menu on create space button click', () => {
const component = getComponent(); render(<SpacePanel />);
fireEvent.click(screen.getByTestId("create-space-button"));
await act(async () => { screen.getByTestId("create-space-button");
findByTestId(component, 'create-space-button').at(0).simulate('click');
component.setProps({});
});
expect(component.find('SpaceCreateMenu').length).toBeTruthy();
}); });
}); });
}); });