Refactoring readMarkerForEvent into ReadMarkerView in shared-components (#32777)

* Refactoring readMarkerForEvent into ReadMarketView in shared-components * Use shared ReadMarkerView in MessagePanel * Rename ReadMarkerView to ReadMarker * Fix Prettier * Update snapshots screenshots * Use plain props for ReadMarker * Fix Prettier * Move ReadMarker into room timeline * Replace ReadMarker nested ternary * Update snapshot
2026-04-14 13:56:52 +02:00
parent 9a8ffbe0bd
commit f615968835
14 changed files with 330 additions and 65 deletions
@@ -68,7 +68,6 @@
@import "./structures/_LeftPanel.pcss";
@import "./structures/_MainSplit.pcss";
@import "./structures/_MatrixChat.pcss";
-@import "./structures/_MessagePanel.pcss";
@import "./structures/_NonUrgentToastContainer.pcss";
@import "./structures/_PictureInPictureDragger.pcss";
@import "./structures/_QuickSettingsButton.pcss";
@@ -1,29 +0,0 @@
-/*
-Copyright 2024 New Vector Ltd.
-Copyright 2023 The Matrix.org Foundation C.I.C.
-
-SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
-Please see LICENSE files in the repository root for full details.
-*/
-
-.mx_MessagePanel_myReadMarker {
-    height: 0;
-    margin: 0;
-    padding: 0;
-    border: 0;
-
-    hr {
-        border-top: solid 1px $accent;
-        border-bottom: solid 1px $accent;
-        margin-top: 0;
-        position: relative;
-        top: -1px;
-        z-index: 1;
-        will-change: width;
-        transition:
-            width 400ms easeinsine 1s,
-            opacity 400ms easeinsine 1s;
-        width: 99%;
-        opacity: 1;
-    }
-}
@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */

-import React, { type JSX, createRef, type ReactNode, type TransitionEvent } from "react";
+import React, { type JSX, createRef, type ReactNode, type TransitionEventHandler } from "react";
 import classNames from "classnames";
 import {
    type Room,
@@ -20,6 +20,7 @@ import { logger } from "matrix-js-sdk/src/logger";
 import { isSupportedReceiptType } from "matrix-js-sdk/src/utils";
 import {
    DateSeparatorView,
+    ReadMarker,
    TimelineSeparator,
    useCreateAutoDisposedViewModel,
 } from "@element-hq/web-shared-components";
@@ -271,7 +272,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
    private readonly _showHiddenEvents: boolean;
    private unmounted = false;

-    private readMarkerNode = createRef<HTMLLIElement>();
+    private readMarkerNode: HTMLLIElement | null = null;
    private whoIsTyping = createRef<WhoIsTypingTile>();
    public scrollPanel = createRef<ScrollPanel>();

@@ -403,7 +404,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
    //   0: read marker is within the window
    //  +1: read marker is below the window
    public getReadMarkerPosition(): number | null {
-        const readMarker = this.readMarkerNode.current;
+        const readMarker = this.readMarkerNode;
        const messageWrapper = this.scrollPanel.current?.divScroll;

        if (!readMarker || !messageWrapper) {
@@ -507,29 +508,18 @@ export default class MessagePanel extends React.Component<IProps, IState> {
    public readMarkerForEvent(eventId: string, isLastEvent: boolean): ReactNode {
        if (this.context.timelineRenderingType === TimelineRenderingType.File) return null;

-        const visible = !isLastEvent && this.props.readMarkerVisible;
+        const showLine = !isLastEvent && !!this.props.readMarkerVisible;

        if (this.props.readMarkerEventId === eventId) {
-            let hr;
-            // if the read marker comes at the end of the timeline (except
-            // for local echoes, which are excluded from RMs, because they
-            // don't have useful event ids), we don't want to show it, but
-            // we still want to create the <li/> for it so that the
-            // algorithms which depend on its position on the screen aren't
-            // confused.
-            if (visible) {
-                hr = <hr style={{ opacity: 1, width: "99%" }} />;
-            }
-
            return (
-                <li
+                <ReadMarker
                    key={"readMarker_" + eventId}
-                    ref={this.readMarkerNode}
+                    eventId={eventId}
+                    kind="current"
+                    showLine={showLine}
+                    onCurrentMarkerRef={this.collectReadMarker}
                    className="mx_MessagePanel_myReadMarker"
-                    data-scroll-tokens={eventId}
-                >
-                    {hr}
-                </li>
+                />
            );
        } else if (this.state.ghostReadMarkers.includes(eventId)) {
            // We render 'ghost' read markers in the DOM while they
@@ -542,28 +532,30 @@ export default class MessagePanel extends React.Component<IProps, IState> {
            // case is a little more complex because only some of the items
            // transition (ie. the read markers do but the event tiles do not)
            // and TransitionGroup requires that all its children are Transitions.
-            const hr = (
-                <hr
-                    ref={this.collectGhostReadMarker}
-                    onTransitionEnd={this.onGhostTransitionEnd}
-                    data-eventid={eventId}
-                />
-            );
-
            // give it a key which depends on the event id. That will ensure that
            // we get a new DOM node (restarting the animation) when the ghost
            // moves to a different event.
            return (
-                <li key={"_readuptoghost_" + eventId} className="mx_MessagePanel_myReadMarker">
-                    {hr}
-                </li>
+                <ReadMarker
+                    key={"_readuptoghost_" + eventId}
+                    eventId={eventId}
+                    kind="ghost"
+                    showLine={true}
+                    onGhostLineRef={this.collectGhostReadMarker}
+                    onGhostTransitionEnd={this.onGhostTransitionEnd}
+                    className="mx_MessagePanel_myReadMarker"
+                />
            );
        }

        return null;
    }

-    private collectGhostReadMarker = (node: HTMLElement | null): void => {
+    private collectReadMarker = (node: HTMLLIElement | null): void => {
+        this.readMarkerNode = node;
+    };
+
+    private collectGhostReadMarker = (node: HTMLHRElement | null): void => {
        if (node) {
            // now the element has appeared, change the style which will trigger the CSS transition
            requestAnimationFrame(() => {
@@ -573,7 +565,7 @@ export default class MessagePanel extends React.Component<IProps, IState> {
        }
    };

-    private onGhostTransitionEnd = (ev: TransitionEvent): void => {
+    private onGhostTransitionEnd: TransitionEventHandler<HTMLHRElement> = (ev): void => {
        // we can now clean up the ghost element
        const finishedEventId = (ev.target as HTMLElement).dataset.eventid;
        this.setState({
@@ -13,6 +13,7 @@ export * from "./core/AvatarWithDetails";
 export * from "./composer/Banner";
 export * from "./crypto/SasEmoji";
 export * from "./event-tiles/UrlPreviewGroupView";
+export * from "./room/timeline/ReadMarker";
 export * from "./room/timeline/event-tile/body/EventContentBodyView";
 export * from "./room/timeline/event-tile/body/RedactedBodyView";
 export * from "./room/timeline/event-tile/body/MFileBodyView";
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+.readMarker {
+    height: 0;
+    margin: 0;
+    padding: 0;
+    border: 0;
+}
+
+.line {
+    margin-top: 0;
+    position: relative;
+    top: -1px;
+    z-index: 1;
+    width: 99%;
+    opacity: 1;
+    border-top: solid 1px var(--cpd-color-icon-accent-tertiary);
+    border-bottom: solid 1px var(--cpd-color-icon-accent-tertiary);
+    will-change: width, opacity;
+    transition:
+        width 400ms easeinsine 1s,
+        opacity 400ms easeinsine 1s;
+}
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import React, { type JSX } from "react";
+import { fn } from "storybook/test";
+
+import type { Meta, StoryObj } from "@storybook/react-vite";
+import { ReadMarker, type ReadMarkerProps } from "./ReadMarker";
+
+const ReadMarkerWrapper = ({
+    className,
+    onCurrentMarkerRef,
+    onGhostLineRef,
+    onGhostTransitionEnd,
+    ...props
+}: Readonly<ReadMarkerProps>): JSX.Element => {
+    return (
+        <ul>
+            <ReadMarker
+                {...props}
+                className={className}
+                onCurrentMarkerRef={onCurrentMarkerRef ?? fn()}
+                onGhostLineRef={onGhostLineRef ?? fn()}
+                onGhostTransitionEnd={onGhostTransitionEnd ?? fn()}
+            />
+        </ul>
+    );
+};
+
+const meta = {
+    title: "Timeline/ReadMarker",
+    component: ReadMarkerWrapper,
+    tags: ["autodocs"],
+    args: {
+        eventId: "$event",
+        kind: "current",
+        showLine: true,
+    },
+} satisfies Meta<typeof ReadMarkerWrapper>;
+
+export default meta;
+type Story = StoryObj<typeof meta>;
+
+export const Current: Story = {};
+
+export const HiddenCurrent: Story = {
+    args: {
+        showLine: false,
+    },
+};
+
+export const Ghost: Story = {
+    args: {
+        kind: "ghost",
+    },
+};
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import { composeStories } from "@storybook/react-vite";
+import { fireEvent, render, screen } from "@test-utils";
+import React from "react";
+import { describe, expect, it, vi } from "vitest";
+
+import { ReadMarker } from "./ReadMarker";
+import * as stories from "./ReadMarker.stories";
+
+const { Current, HiddenCurrent, Ghost } = composeStories(stories);
+
+describe("ReadMarker", () => {
+    it("renders the current read marker", () => {
+        const { container } = render(<Current />);
+        expect(container).toMatchSnapshot();
+    });
+
+    it("renders the hidden current read marker without a line", () => {
+        const { container } = render(<HiddenCurrent />);
+        expect(container).toMatchSnapshot();
+        expect(container.querySelector("hr")).toBeNull();
+    });
+
+    it("renders the ghost read marker", () => {
+        const { container } = render(<Ghost />);
+        expect(container).toMatchSnapshot();
+    });
+
+    it("applies custom className to the list item", () => {
+        render(
+            <ul>
+                <ReadMarker
+                    eventId="$event"
+                    kind="current"
+                    showLine={true}
+                    className="custom-read-marker compatibility-class"
+                />
+            </ul>,
+        );
+
+        const item = screen.getByRole("listitem");
+        expect(item).toHaveClass("custom-read-marker", "compatibility-class");
+        expect(item).toHaveAttribute("data-scroll-tokens", "$event");
+    });
+
+    it("wires ghost marker actions", () => {
+        const onGhostLineRef = vi.fn();
+        const onGhostTransitionEnd = vi.fn();
+
+        render(
+            <ul>
+                <ReadMarker
+                    eventId="$ghost"
+                    kind="ghost"
+                    onGhostLineRef={onGhostLineRef}
+                    onGhostTransitionEnd={onGhostTransitionEnd}
+                />
+            </ul>,
+        );
+
+        const line = screen.getByRole("separator");
+        fireEvent.transitionEnd(line);
+
+        expect(onGhostLineRef).toHaveBeenCalled();
+        expect(onGhostTransitionEnd).toHaveBeenCalledTimes(1);
+    });
+
+    it("wires the current marker ref", () => {
+        const onCurrentMarkerRef = vi.fn();
+
+        render(
+            <ul>
+                <ReadMarker eventId="$current" kind="current" showLine={true} onCurrentMarkerRef={onCurrentMarkerRef} />
+            </ul>,
+        );
+
+        expect(onCurrentMarkerRef).toHaveBeenCalled();
+    });
+});
@@ -0,0 +1,80 @@
+/*
+ * Copyright 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import React, { type JSX, type RefCallback, type TransitionEventHandler } from "react";
+import classNames from "classnames";
+
+import styles from "./ReadMarker.module.css";
+
+export type ReadMarkerKind = "current" | "ghost";
+
+export interface ReadMarkerProps {
+    /**
+     * The event ID this marker is associated with.
+     */
+    eventId: string;
+    /**
+     * Whether this is the active read marker or a ghost marker transitioning out.
+     */
+    kind: ReadMarkerKind;
+    /**
+     * Whether the visible line should be rendered for the active marker.
+     * Hidden active markers still render the host `<li>` to preserve layout calculations.
+     */
+    showLine?: boolean;
+    /**
+     * Ref callback for the active read marker `<li>`.
+     */
+    onCurrentMarkerRef?: RefCallback<HTMLLIElement>;
+    /**
+     * Ref callback for the ghost marker `<hr>`.
+     */
+    onGhostLineRef?: RefCallback<HTMLHRElement>;
+    /**
+     * Transition-end handler for the ghost marker `<hr>`.
+     */
+    onGhostTransitionEnd?: TransitionEventHandler<HTMLHRElement>;
+    /**
+     * Optional CSS className for the outer list item.
+     */
+    className?: string;
+}
+
+export function ReadMarker({
+    eventId,
+    kind,
+    showLine = true,
+    onCurrentMarkerRef,
+    onGhostLineRef,
+    onGhostTransitionEnd,
+    className,
+}: Readonly<ReadMarkerProps>): JSX.Element {
+    let line: JSX.Element | null = null;
+
+    if (kind === "ghost") {
+        line = (
+            <hr
+                className={styles.line}
+                ref={onGhostLineRef}
+                onTransitionEnd={onGhostTransitionEnd}
+                data-eventid={eventId}
+            />
+        );
+    } else if (showLine) {
+        line = <hr className={styles.line} />;
+    }
+
+    return (
+        <li
+            className={classNames(className, styles.readMarker)}
+            ref={kind === "current" ? onCurrentMarkerRef : undefined}
+            data-scroll-tokens={kind === "current" ? eventId : undefined}
+        >
+            {line}
+        </li>
+    );
+}
@@ -0,0 +1,42 @@
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+exports[`ReadMarker > renders the current read marker 1`] = `
+<div>
+  <ul>
+    <li
+      class="ReadMarker-module_readMarker"
+      data-scroll-tokens="$event"
+    >
+      <hr
+        class="ReadMarker-module_line"
+      />
+    </li>
+  </ul>
+</div>
+`;
+
+exports[`ReadMarker > renders the ghost read marker 1`] = `
+<div>
+  <ul>
+    <li
+      class="ReadMarker-module_readMarker"
+    >
+      <hr
+        class="ReadMarker-module_line"
+        data-eventid="$event"
+      />
+    </li>
+  </ul>
+</div>
+`;
+
+exports[`ReadMarker > renders the hidden current read marker without a line 1`] = `
+<div>
+  <ul>
+    <li
+      class="ReadMarker-module_readMarker"
+      data-scroll-tokens="$event"
+    />
+  </ul>
+</div>
+`;
@@ -0,0 +1,8 @@
+/*
+ * Copyright 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+export { ReadMarker, type ReadMarkerKind, type ReadMarkerProps } from "./ReadMarker";
@@ -1 +0,0 @@
-