Fix issues with the new topic dialog (#8608)

Author: SimonBrandner

res/css/views/rooms/_RoomHeader.scss

@@ -142,6 +142,7 @@ limitations under the License.
 
 .mx_RoomTopic {
     position: relative;
+    cursor: pointer;
 }
 
 .mx_RoomHeader_topic {

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,
     });
 }
+

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<HTMLDivElement> {
     room?: Room;
@@ -43,7 +42,6 @@ export default function RoomTopic({
 }: IProps) {
     const client = useContext(MatrixClientContext);
     const ref = useRef<HTMLDivElement>();
-    const hovered = useHover(ref);
 
     const topic = useTopic(room);
 
@@ -57,14 +55,27 @@ 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());
 
             const modal = Modal.createDialog(InfoDialog, {
                 title: room.name,
                 description: <div>
-                    <Linkify as="p">{ topic }</Linkify>
+                    <Linkify
+                        as="p"
+                        onClick={(ev: MouseEvent) => {
+                            if ((ev.target as HTMLElement).tagName.toUpperCase() === "A") {
+                                modal.close();
+                            }
+                        }}
+                    >
+                        { topic }
+                    </Linkify>
                     { canSetTopic && <AccessibleButton
                         kind="primary_outline"
                         onClick={() => {
@@ -80,10 +91,6 @@ export default function RoomTopic({
         }
     });
 
-    useEffect(() => {
-        linkifyElement(ref.current);
-    }, [topic]);
-
     const className = classNames(props.className, "mx_RoomTopic");
 
     return <div {...props}
@@ -92,9 +99,10 @@ export default function RoomTopic({
         dir="auto"
         className={className}
     >
-        { topic }
-        { hovered && (
-            <Tooltip label={_t("Click to read topic")} alignment={Alignment.Bottom} />
-        ) }
+        <TooltipTarget label={_t("Click to read topic")} alignment={Alignment.Bottom} ignoreHover={ignoreHover}>
+            <Linkify>
+                { topic }
+            </Linkify>
+        </TooltipTarget>
     </div>;
 }

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<HTMLSpanElement>, Omit<ITooltipProps, 'visible'> {
     tooltipTargetClassName?: string;
+    ignoreHover?: (ev: React.MouseEvent) => boolean;
 }
 
 /**
@@ -36,34 +39,31 @@ const TooltipTarget: React.FC<IProps> = ({
     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 && <Tooltip
+    const tooltip = (isFocused || isHovering) && <Tooltip
         id={id}
         className={className}
         tooltipClassName={tooltipClassName}
         label={label}
         alignment={alignment}
-        visible={isVisible}
+        visible={isFocused || isHovering}
         maxParentWidth={maxParentWidth}
     />;
 
     return (
         <div
+            {...hoverProps}
+            {...focusProps}
             tabIndex={0}
             aria-describedby={id}
             className={tooltipTargetClassName}
-            onMouseOver={show}
-            onMouseLeave={hide}
-            onFocus={show}
-            onBlur={hide}
             {...rest}
         >
             { children }

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];
+}

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<HTMLElement>) {
+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];
 }

test/components/views/elements/Linkify-test.tsx

@@ -22,7 +22,23 @@ describe("Linkify", () => {
         const wrapper = mount(<Linkify>
             https://perdu.com
         </Linkify>);
-        expect(wrapper.html()).toBe('<div><a href="https://perdu.com">https://perdu.com</a></div>');
+        expect(wrapper.html()).toBe(
+            "<div><a href=\"https://perdu.com\" class=\"linkified\" target=\"_blank\" rel=\"noreferrer noopener\">"+
+            "https://perdu.com" +
+            "</a></div>",
+        );
+    });
+
+    it("correctly linkifies a room alias", () => {
+        const wrapper = mount(<Linkify>
+            #element-web:matrix.org
+        </Linkify>);
+        expect(wrapper.html()).toBe(
+            "<div>" +
+            "<a href=\"https://matrix.to/#/#element-web:matrix.org\" class=\"linkified\" rel=\"noreferrer noopener\">" +
+            "#element-web:matrix.org" +
+            "</a></div>",
+        );
     });
 
     it("changes the root tag name", () => {
@@ -55,10 +71,20 @@ describe("Linkify", () => {
 
         const wrapper = mount(<DummyTest />);
 
-        expect(wrapper.html()).toBe('<div><div><a href="https://perdu.com">https://perdu.com</a></div></div>');
+        expect(wrapper.html()).toBe(
+            "<div><div>" +
+            "<a href=\"https://perdu.com\" class=\"linkified\" target=\"_blank\" rel=\"noreferrer noopener\">" +
+            "https://perdu.com" +
+            "</a></div></div>",
+        );
 
         wrapper.find('div').at(0).simulate('click');
 
-        expect(wrapper.html()).toBe('<div><div><a href="https://matrix.org">https://matrix.org</a></div></div>');
+        expect(wrapper.html()).toBe(
+            "<div><div>" +
+            "<a href=\"https://matrix.org\" class=\"linkified\" target=\"_blank\" rel=\"noreferrer noopener\">" +
+            "https://matrix.org" +
+            "</a></div></div>",
+        );
     });
 });

test/components/views/messages/__snapshots__/MLocationBody-test.tsx.snap

@@ -95,6 +95,7 @@ exports[`MLocationBody <MLocationBody> without error renders map correctly 1`] =
           onBlur={[Function]}
           onFocus={[Function]}
           onMouseLeave={[Function]}
+          onMouseMove={[Function]}
           onMouseOver={[Function]}
           tabIndex={0}
         >