Skip to content

Refactor EventPreview to shared MVVM#33646

Open
ZacksBot wants to merge 7 commits into
element-hq:developfrom
ZacksBot:refactor/event-preview-mvvm
Open

Refactor EventPreview to shared MVVM#33646
ZacksBot wants to merge 7 commits into
element-hq:developfrom
ZacksBot:refactor/event-preview-mvvm

Conversation

@ZacksBot
Copy link
Copy Markdown
Member

@ZacksBot ZacksBot commented May 28, 2026

Refactor EventPreview into shared component using MVVM pattern and remove legacy component

Fixes: https://github.com/element-hq/wat-internal/issues/486

@ZacksBot ZacksBot self-assigned this May 28, 2026
@ZacksBot ZacksBot added the T-Task Tasks for the team like planning label May 28, 2026

this.disposables.track(() => this.eventContentListener.teardown());
this.eventContentListener.setEvent(props.mxEvent, this.onEventContentChanged);
void this.updatePreview();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also might benefit from being in a catch block (and wrapper, probably)?


private async updatePreview(): Promise<void> {
const { cli, mxEvent } = this.props;
const requestId = ++this.previewRequestId;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll only actually need to wait for the event to be decrypted once here in practice so you could also just set an 'updating preview' flag and return if it's set, or it may not even matter at all. That said, this should work fine too, so doesn't really matter.

const mxEvent = makeMessageEvent({ body: "Text preview" });
const vm = new EventPreviewViewModel({ cli, mxEvent });

await flushPromises();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this isn't marked deprecated then it should be: it's much better to find something to waitFor instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Task Tasks for the team like planning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants