From 74734036fd8b6c147019937a3f1ab538844ddc95 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 14 Oct 2022 09:06:05 +0100 Subject: [PATCH 1/5] Prevent useContextMenu isOpen from being true if the button ref goes away --- src/components/structures/ContextMenu.tsx | 2 +- src/components/views/rooms/ReadReceiptGroup.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 9a13d624248..9863a5d01f5 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -579,7 +579,7 @@ export const useContextMenu = (): ContextMenuTuple< setIsOpen(false); }; - return [isOpen, button, open, close, setIsOpen]; + return [button.current && isOpen, button, open, close, setIsOpen]; }; // XXX: Deprecated, used only for dynamic Tooltips. Avoid using at all costs. diff --git a/src/components/views/rooms/ReadReceiptGroup.tsx b/src/components/views/rooms/ReadReceiptGroup.tsx index 34bcb431e29..4e4b4c8a9dd 100644 --- a/src/components/views/rooms/ReadReceiptGroup.tsx +++ b/src/components/views/rooms/ReadReceiptGroup.tsx @@ -209,7 +209,8 @@ export function ReadReceiptGroup( onMouseOver={showTooltip} onMouseLeave={hideTooltip} onFocus={showTooltip} - onBlur={hideTooltip}> + onBlur={hideTooltip} + > { remText } Date: Mon, 17 Oct 2022 10:08:56 +0100 Subject: [PATCH 2/5] Fix tests --- src/components/structures/ContextMenu.tsx | 2 +- src/components/views/spaces/SpacePanel.tsx | 2 +- .../views/spaces/SpacePanel-test.tsx | 30 ++++++------------- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 9863a5d01f5..46223b2114f 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -579,7 +579,7 @@ export const useContextMenu = (): ContextMenuTuple< setIsOpen(false); }; - return [button.current && 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. diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index 129e6f3584e..5f640f52f33 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -231,7 +231,7 @@ const CreateSpaceButton = ({ role="treeitem" > ({ })); describe('', () => { - const defaultProps = {}; - const getComponent = (props = {}) => - mount(); - const mockClient = { getUserId: jest.fn().mockReturnValue('@test:test'), isGuest: jest.fn(), @@ -67,26 +60,21 @@ describe('', () => { describe('create new space button', () => { it('renders create space button when UIComponent.CreateSpaces component should be shown', () => { - const component = getComponent(); - expect(findByTestId(component, 'create-space-button').length).toBeTruthy(); + render(); + screen.getByTestId("create-space-button"); }); it('does not render create space button when UIComponent.CreateSpaces component should not be shown', () => { mocked(shouldShowComponent).mockReturnValue(false); - const component = getComponent(); + render(); 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 () => { - const component = getComponent(); - - await act(async () => { - findByTestId(component, 'create-space-button').at(0).simulate('click'); - component.setProps({}); - }); - - expect(component.find('SpaceCreateMenu').length).toBeTruthy(); + it('opens context menu on create space button click', () => { + render(); + fireEvent.click(screen.getByTestId("create-space-button")); + screen.getByTestId("create-space-button"); }); }); }); From b6d5da60c56917e0ab46a839f18d27d82f3501b1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 17 Oct 2022 15:27:27 +0100 Subject: [PATCH 3/5] Fix context menu ref passing --- src/components/structures/ContextMenu.tsx | 9 ++++++-- src/components/views/spaces/SpacePanel.tsx | 5 ++--- .../views/spaces/SpaceTreeLevel.tsx | 21 +++++++++++++------ 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 46223b2114f..b62c4b6d7be 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -565,8 +565,13 @@ type ContextMenuTuple = [ (val: boolean) => void, ]; // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint -export const useContextMenu = (): ContextMenuTuple => { - const button = useRef(null); +export const useContextMenu = (inputRef?: RefObject): ContextMenuTuple => { + let button = useRef(null); + if (inputRef) { + // if we are given a ref, use it instead of ours + button = inputRef; + } + const [isOpen, setIsOpen] = useState(false); const open = (ev?: SyntheticEvent) => { ev?.preventDefault(); diff --git a/src/components/views/spaces/SpacePanel.tsx b/src/components/views/spaces/SpacePanel.tsx index 5f640f52f33..e26a78b5a37 100644 --- a/src/components/views/spaces/SpacePanel.tsx +++ b/src/components/views/spaces/SpacePanel.tsx @@ -204,9 +204,7 @@ const CreateSpaceButton = ({ isPanelCollapsed, setPanelCollapsed, }: Pick) => { - // We don't need the handle as we position the menu in a constant location - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const [menuDisplayed, _handle, openMenu, closeMenu] = useContextMenu(); + const [menuDisplayed, handle, openMenu, closeMenu] = useContextMenu(); useEffect(() => { if (!isPanelCollapsed && menuDisplayed) { @@ -238,6 +236,7 @@ const CreateSpaceButton = ({ label={menuDisplayed ? _t("Cancel") : _t("Create a space")} onClick={onNewClick} isNarrow={isPanelCollapsed} + ref={handle} /> { contextMenu } diff --git a/src/components/views/spaces/SpaceTreeLevel.tsx b/src/components/views/spaces/SpaceTreeLevel.tsx index cab7bc3c76b..5952e877d9f 100644 --- a/src/components/views/spaces/SpaceTreeLevel.tsx +++ b/src/components/views/spaces/SpaceTreeLevel.tsx @@ -14,7 +14,16 @@ See the License for the specific language governing permissions and 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 { Room, RoomEvent } from "matrix-js-sdk/src/models/room"; import { DraggableProvidedDragHandleProps } from "react-beautiful-dnd"; @@ -54,7 +63,7 @@ interface IButtonProps extends Omit = ({ +export const SpaceButton = forwardRef(({ space, spaceKey, className, @@ -67,9 +76,9 @@ export const SpaceButton: React.FC = ({ children, ContextMenuComponent, ...props -}) => { - const [menuDisplayed, ref, openMenu, closeMenu] = useContextMenu(); - const [onFocus, isActive, handle] = useRovingTabIndex(ref); +}, ref: RefObject) => { + const [menuDisplayed, handle, openMenu, closeMenu] = useContextMenu(ref); + const [onFocus, isActive] = useRovingTabIndex(handle); const tabIndex = isActive ? 0 : -1; let avatar =
; @@ -150,7 +159,7 @@ export const SpaceButton: React.FC = ({
); -}; +}); interface IItemProps extends InputHTMLAttributes { space: Room; From 30ad1065d5fe9e3f99e2708f6bd6776bd71a4d58 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 17 Oct 2022 17:10:00 +0100 Subject: [PATCH 4/5] Fix missing useContextMenu ref usage --- src/components/views/location/LocationButton.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/location/LocationButton.tsx b/src/components/views/location/LocationButton.tsx index 7efd350b6b9..2460e8463dc 100644 --- a/src/components/views/location/LocationButton.tsx +++ b/src/components/views/location/LocationButton.tsx @@ -69,6 +69,7 @@ export const LocationButton: React.FC = ({ roomId, sender, menuPosition, iconClassName="mx_MessageComposer_location" onClick={openMenu} title={_t("Location")} + innerRef={button} /> { contextMenu } From 38625e3411421f255eade723d5a52312f1bc86e9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 17 Oct 2022 17:12:31 +0100 Subject: [PATCH 5/5] Fix prop name --- src/components/views/location/LocationButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/location/LocationButton.tsx b/src/components/views/location/LocationButton.tsx index 2460e8463dc..6d9885cf915 100644 --- a/src/components/views/location/LocationButton.tsx +++ b/src/components/views/location/LocationButton.tsx @@ -69,7 +69,7 @@ export const LocationButton: React.FC = ({ roomId, sender, menuPosition, iconClassName="mx_MessageComposer_location" onClick={openMenu} title={_t("Location")} - innerRef={button} + inputRef={button} /> { contextMenu }