From 3b0c04c2e97bfc3d97619a55d818a6e5e72092ec Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 17 Jul 2025 13:32:31 +0100 Subject: [PATCH] Add SubscriptionViewModel base class (#30297) * Very first pass at shared component views Turn the trivial TextualEvent into a shared component with a separate view model for element web. Args to view model will probably change to be more specific and VM typer needs abstracting out into an interface, but should give the general idea. * Remove old TextualEvent * Pass showHiddenEvents Because we used it anyway, we just cheated by getting it from the context * Factor out common view model stuff * Move ViewModel interface into the shared components * Add tiny wrapper hook * Move showHiddenEvents into props fully * Fill in stories / test * chore: setup storybook cherry pick edc5e8705674b8708d986910b02b5d2545777fb3 from florianduros/storybook * Add TextualEvent component to storybook * Add mock view model & snapshot * Remove old style stories entry * Change import * Change import * Prettier * Add paxckage patch to @types/mdx for React 19 compat * Pass getSnapshot as getServerSnapshot too * Maybe make sonar regognise tests as tests * Typo * Use storybook reacvt-vite There's no reason to use the react-webpack plugin just because our app is stuck on webpack, it just means we have vite as a dependency too. * Change here too * Workaround for incomatible types in rollup https://github.com/rollup/rollup/issues/5199 * Remove webpack styling addon Not necessary now we're using vite * Hopefully do screenshot testing... * need newer node * quote issues * Make it an npm script * colons * use right port * Install playwright browsers * Try without the if * Oh right, we need the headless shell * Pass flag to store received screenshots and upload diffs too * Update snapshot from received * Include platform in snapshot / received dir because font rendering differs between platforms * Suffix snapshots with platform instead like we do for playwright * Remove unnecessary env vars and better name * Add some comments * Prettier * Fix yarn.lock * Memoise vm creation Co-authored-by: Florian Duros * Add implements Co-authored-by: Florian Duros * Fix listener interface * Add implements Co-authored-by: Florian Duros * Fix types * Fix more types * Add a superclass that simple view models can extend to reduce boilerplate * Revert useMemo as this isn't a hook * Unused import * Actually commit the file the branch is named after * Add missing playwright step * Add return type annotation * Change to add / remove subscription callback * Change to 'add' rather than 'subs.subscribe' * Add cache specifier for only shell playwright browsers * Add copyright headers * Better comment wording * Make amit an arrow function so it can be passed directly as a callback * Add a test --------- Co-authored-by: Florian Duros Co-authored-by: Florian Duros --- src/viewmodels/SubscriptionViewModel.ts | 56 +++++++++++++++++++ src/viewmodels/ViewModelSubscriptions.ts | 15 ++--- .../event-tiles/TextualEventViewModel.ts | 25 +++------ .../event-tiles/TextualEventViewModel-test.ts | 29 ++++++++++ 4 files changed, 96 insertions(+), 29 deletions(-) create mode 100644 src/viewmodels/SubscriptionViewModel.ts create mode 100644 test/viewmodels/event-tiles/TextualEventViewModel-test.ts diff --git a/src/viewmodels/SubscriptionViewModel.ts b/src/viewmodels/SubscriptionViewModel.ts new file mode 100644 index 0000000000..65147d4cc4 --- /dev/null +++ b/src/viewmodels/SubscriptionViewModel.ts @@ -0,0 +1,56 @@ +/* +Copyright 2025 New Vector 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 { type ViewModel } from "../shared-components/ViewModel"; +import { ViewModelSubscriptions } from "./ViewModelSubscriptions"; + +export abstract class SubscriptionViewModel implements ViewModel { + protected subs: ViewModelSubscriptions; + + protected constructor() { + this.subs = new ViewModelSubscriptions( + this.addDownstreamSubscriptionWrapper, + this.removeDownstreamSubscriptionWrapper, + ); + } + + public subscribe = (listener: () => void): (() => void) => { + return this.subs.add(listener); + }; + + /** + * Wrapper around the abstract subscribe callback as we can't assume that the subclassed method + * has a bound `this` context. + */ + private addDownstreamSubscriptionWrapper = (): void => { + this.addDownstreamSubscription(); + }; + + /** + * Wrapper around the abstract unsubscribe callback as we can't call pass an abstract method directly + * in the constructor. + */ + private removeDownstreamSubscriptionWrapper = (): void => { + this.removeDownstreamSubscription(); + }; + + /** + * Called when the first listener subscribes: the subclass should set up any necessary subscriptions + * to call this.subs.emit() when the snapshot changes. + */ + protected abstract addDownstreamSubscription(): void; + + /** + * Called when the last listener unsubscribes: the subclass should clean up any subscriptions. + */ + protected abstract removeDownstreamSubscription(): void; + + /** + * Returns the current snapshot of the view model. + */ + public abstract getSnapshot: () => T; +} diff --git a/src/viewmodels/ViewModelSubscriptions.ts b/src/viewmodels/ViewModelSubscriptions.ts index ffbc89b6f1..97373fcf9f 100644 --- a/src/viewmodels/ViewModelSubscriptions.ts +++ b/src/viewmodels/ViewModelSubscriptions.ts @@ -12,7 +12,8 @@ export class ViewModelSubscriptions { private listeners = new Set<() => void>(); /** - * @param updateSubscription A function called whenever a listener is added or removed. + * @param subscribeCallback Called when the first listener subscribes. + * @param unsubscribeCallback Called when the last listener unsubscribes. */ public constructor( private subscribeCallback: () => void, @@ -41,17 +42,9 @@ export class ViewModelSubscriptions { /** * Emit an update to all subscribed listeners. */ - public emit(): void { + public emit = (): void => { for (const listener of this.listeners) { listener(); } - } - - /** - * Get the number of listeners currently subscribed to updates. - * @returns The number of listeners. - */ - public listenerCount(): number { - return this.listeners.size; - } + }; } diff --git a/src/viewmodels/event-tiles/TextualEventViewModel.ts b/src/viewmodels/event-tiles/TextualEventViewModel.ts index 9d8477b575..d2f56482d7 100644 --- a/src/viewmodels/event-tiles/TextualEventViewModel.ts +++ b/src/viewmodels/event-tiles/TextualEventViewModel.ts @@ -10,27 +10,20 @@ import { MatrixEventEvent } from "matrix-js-sdk/src/matrix"; import { type EventTileTypeProps } from "../../events/EventTileFactory"; import { MatrixClientPeg } from "../../MatrixClientPeg"; import { textForEvent } from "../../TextForEvent"; -import { ViewModelSubscriptions } from "../ViewModelSubscriptions"; import { type TextualEventViewSnapshot } from "../../shared-components/event-tiles/TextualEvent/TextualEvent"; -import { type ViewModel } from "../../shared-components/ViewModel"; - -export class TextualEventViewModel implements ViewModel { - private subs: ViewModelSubscriptions; +import { SubscriptionViewModel } from "../SubscriptionViewModel"; +export class TextualEventViewModel extends SubscriptionViewModel { public constructor(private eventTileProps: EventTileTypeProps) { - this.subs = new ViewModelSubscriptions(this.addSubscription, this.removeSubscription); + super(); } - private addSubscription = (): void => { - this.eventTileProps.mxEvent.on(MatrixEventEvent.SentinelUpdated, this.onEventSentinelUpdated); + protected addDownstreamSubscription = (): void => { + this.eventTileProps.mxEvent.on(MatrixEventEvent.SentinelUpdated, this.subs.emit); }; - private removeSubscription = (): void => { - this.eventTileProps.mxEvent.off(MatrixEventEvent.SentinelUpdated, this.onEventSentinelUpdated); - }; - - public subscribe = (listener: () => void): (() => void) => { - return this.subs.add(listener); + protected removeDownstreamSubscription = (): void => { + this.eventTileProps.mxEvent.off(MatrixEventEvent.SentinelUpdated, this.subs.emit); }; public getSnapshot = (): TextualEventViewSnapshot => { @@ -42,8 +35,4 @@ export class TextualEventViewModel implements ViewModel { - this.subs.emit(); - }; } diff --git a/test/viewmodels/event-tiles/TextualEventViewModel-test.ts b/test/viewmodels/event-tiles/TextualEventViewModel-test.ts new file mode 100644 index 0000000000..92037df9fc --- /dev/null +++ b/test/viewmodels/event-tiles/TextualEventViewModel-test.ts @@ -0,0 +1,29 @@ +/* +Copyright 2025 New Vector 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 { MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/matrix"; + +import { TextualEventViewModel } from "../../../src/viewmodels/event-tiles/TextualEventViewModel"; + +describe("TextualEventViewModel", () => { + it("should update when the sentinel updates", () => { + const fakeEvent = new MatrixEvent({}); + + const vm = new TextualEventViewModel({ + showHiddenEvents: false, + mxEvent: fakeEvent, + }); + + const cb = jest.fn(); + + vm.subscribe(cb); + + fakeEvent.emit(MatrixEventEvent.SentinelUpdated); + + expect(cb).toHaveBeenCalledTimes(1); + }); +});