From fb30b67b14a885e9f1b9e9e5d9e51947ca55c98f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Mon, 16 May 2022 14:10:00 +0200 Subject: [PATCH] Fix issues with the new topic dialog (#8608) --- res/css/views/rooms/_RoomHeader.scss | 1 + src/components/views/elements/Linkify.tsx | 11 ++++-- src/components/views/elements/RoomTopic.tsx | 36 +++++++++++-------- .../views/elements/TooltipTarget.tsx | 22 ++++++------ src/hooks/useFocus.ts | 29 +++++++++++++++ src/hooks/useHover.ts | 31 ++++++---------- .../views/elements/Linkify-test.tsx | 32 +++++++++++++++-- .../__snapshots__/MLocationBody-test.tsx.snap | 1 + 8 files changed, 112 insertions(+), 51 deletions(-) create mode 100644 src/hooks/useFocus.ts diff --git a/res/css/views/rooms/_RoomHeader.scss b/res/css/views/rooms/_RoomHeader.scss index 119bbc90b8..7736ea1cc8 100644 --- a/res/css/views/rooms/_RoomHeader.scss +++ b/res/css/views/rooms/_RoomHeader.scss @@ -142,6 +142,7 @@ limitations under the License. .mx_RoomTopic { position: relative; + cursor: pointer; } .mx_RoomHeader_topic { diff --git a/src/components/views/elements/Linkify.tsx b/src/components/views/elements/Linkify.tsx index 6263df1a12..4d75fb7921 100644 --- a/src/components/views/elements/Linkify.tsx +++ b/src/components/views/elements/Linkify.tsx @@ -14,26 +14,31 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useEffect, useRef } from "react"; -import linkifyElement from "linkify-element"; +import React, { useLayoutEffect, useRef } from "react"; + +import { linkifyElement } from "../../../HtmlUtils"; interface Props { as?: string; children: React.ReactNode; + onClick?: (ev: MouseEvent) => void; } export function Linkify({ as = "div", children, + onClick, }: Props): JSX.Element { const ref = useRef(); - useEffect(() => { + useLayoutEffect(() => { linkifyElement(ref.current); }, [children]); return React.createElement(as, { children, ref, + onClick, }); } + diff --git a/src/components/views/elements/RoomTopic.tsx b/src/components/views/elements/RoomTopic.tsx index a50d3b3308..5cbfb2af27 100644 --- a/src/components/views/elements/RoomTopic.tsx +++ b/src/components/views/elements/RoomTopic.tsx @@ -14,15 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useCallback, useContext, useEffect, useRef } from "react"; +import React, { useCallback, useContext, useRef } from "react"; import { Room } from "matrix-js-sdk/src/models/room"; import classNames from "classnames"; import { EventType } from "matrix-js-sdk/src/@types/event"; -import { linkifyElement } from "../../../HtmlUtils"; import { useTopic } from "../../../hooks/room/useTopic"; -import useHover from "../../../hooks/useHover"; -import Tooltip, { Alignment } from "./Tooltip"; +import { Alignment } from "./Tooltip"; import { _t } from "../../../languageHandler"; import dis from "../../../dispatcher/dispatcher"; import { Action } from "../../../dispatcher/actions"; @@ -32,6 +30,7 @@ import { useDispatcher } from "../../../hooks/useDispatcher"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; import AccessibleButton from "./AccessibleButton"; import { Linkify } from "./Linkify"; +import TooltipTarget from "./TooltipTarget"; interface IProps extends React.HTMLProps { room?: Room; @@ -43,7 +42,6 @@ export default function RoomTopic({ }: IProps) { const client = useContext(MatrixClientContext); const ref = useRef(); - const hovered = useHover(ref); const topic = useTopic(room); @@ -57,6 +55,10 @@ export default function RoomTopic({ dis.fire(Action.ShowRoomTopic); }, [props]); + const ignoreHover = (ev: React.MouseEvent): boolean => { + return (ev.target as HTMLElement).tagName.toUpperCase() === "A"; + }; + useDispatcher(dis, (payload) => { if (payload.action === Action.ShowRoomTopic) { const canSetTopic = room.currentState.maySendStateEvent(EventType.RoomTopic, client.getUserId()); @@ -64,7 +66,16 @@ export default function RoomTopic({ const modal = Modal.createDialog(InfoDialog, { title: room.name, description:
- { topic } + { + if ((ev.target as HTMLElement).tagName.toUpperCase() === "A") { + modal.close(); + } + }} + > + { topic } + { canSetTopic && { @@ -80,10 +91,6 @@ export default function RoomTopic({ } }); - useEffect(() => { - linkifyElement(ref.current); - }, [topic]); - const className = classNames(props.className, "mx_RoomTopic"); return
- { topic } - { hovered && ( - - ) } + + + { topic } + +
; } diff --git a/src/components/views/elements/TooltipTarget.tsx b/src/components/views/elements/TooltipTarget.tsx index 1f6339a072..d300069169 100644 --- a/src/components/views/elements/TooltipTarget.tsx +++ b/src/components/views/elements/TooltipTarget.tsx @@ -14,12 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useState, HTMLAttributes } from 'react'; +import React, { HTMLAttributes } from 'react'; +import useFocus from "../../../hooks/useFocus"; +import useHover from "../../../hooks/useHover"; import Tooltip, { ITooltipProps } from './Tooltip'; interface IProps extends HTMLAttributes, Omit { tooltipTargetClassName?: string; + ignoreHover?: (ev: React.MouseEvent) => boolean; } /** @@ -36,34 +39,31 @@ const TooltipTarget: React.FC = ({ alignment, tooltipClassName, maxParentWidth, + ignoreHover, ...rest }) => { - const [isVisible, setIsVisible] = useState(false); - - const show = () => setIsVisible(true); - const hide = () => setIsVisible(false); + const [isFocused, focusProps] = useFocus(); + const [isHovering, hoverProps] = useHover(ignoreHover); // No need to fill up the DOM with hidden tooltip elements. Only add the // tooltip when we're hovering over the item (performance) - const tooltip = isVisible && ; return (
{ children } diff --git a/src/hooks/useFocus.ts b/src/hooks/useFocus.ts new file mode 100644 index 0000000000..f84bc49be2 --- /dev/null +++ b/src/hooks/useFocus.ts @@ -0,0 +1,29 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { useState } from "react"; + +export default function useFocus( +): [boolean, {onFocus: () => void, onBlur: () => void}] { + const [focused, setFocused] = useState(false); + + const props = { + onFocus: () => setFocused(true), + onBlur: () => setFocused(false), + }; + + return [focused, props]; +} diff --git a/src/hooks/useHover.ts b/src/hooks/useHover.ts index 9f7c101225..644c3a630a 100644 --- a/src/hooks/useHover.ts +++ b/src/hooks/useHover.ts @@ -14,29 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useEffect, useState } from "react"; +import { useState } from "react"; -export default function useHover(ref: React.MutableRefObject) { +export default function useHover( + ignoreHover?: (ev: React.MouseEvent) => boolean, +): [boolean, { onMouseOver: () => void, onMouseLeave: () => void, onMouseMove: (ev: React.MouseEvent) => void }] { const [hovered, setHoverState] = useState(false); - const handleMouseOver = () => setHoverState(true); - const handleMouseOut = () => setHoverState(false); - - useEffect( - () => { - const node = ref.current; - if (node) { - node.addEventListener("mouseover", handleMouseOver); - node.addEventListener("mouseout", handleMouseOut); - - return () => { - node.removeEventListener("mouseover", handleMouseOver); - node.removeEventListener("mouseout", handleMouseOut); - }; - } + const props = { + onMouseOver: () => setHoverState(true), + onMouseLeave: () => setHoverState(false), + onMouseMove: (ev: React.MouseEvent): void => { + setHoverState(!ignoreHover(ev)); }, - [ref], - ); + }; - return hovered; + return [hovered, props]; } diff --git a/test/components/views/elements/Linkify-test.tsx b/test/components/views/elements/Linkify-test.tsx index 7224c54373..f663d4c927 100644 --- a/test/components/views/elements/Linkify-test.tsx +++ b/test/components/views/elements/Linkify-test.tsx @@ -22,7 +22,23 @@ describe("Linkify", () => { const wrapper = mount( https://perdu.com ); - expect(wrapper.html()).toBe(''); + expect(wrapper.html()).toBe( + "", + ); + }); + + it("correctly linkifies a room alias", () => { + const wrapper = mount( + #element-web:matrix.org + ); + expect(wrapper.html()).toBe( + "", + ); }); it("changes the root tag name", () => { @@ -55,10 +71,20 @@ describe("Linkify", () => { const wrapper = mount(); - expect(wrapper.html()).toBe(''); + expect(wrapper.html()).toBe( + "", + ); wrapper.find('div').at(0).simulate('click'); - expect(wrapper.html()).toBe(''); + expect(wrapper.html()).toBe( + "", + ); }); }); diff --git a/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap b/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap index 58a413fdb7..f4914b510d 100644 --- a/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap +++ b/test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap @@ -95,6 +95,7 @@ exports[`MLocationBody without error renders map correctly 1`] = onBlur={[Function]} onFocus={[Function]} onMouseLeave={[Function]} + onMouseMove={[Function]} onMouseOver={[Function]} tabIndex={0} >