Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shorten TimelineWindow when an event is removed #3862

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

andybalaam
Copy link
Member

@andybalaam andybalaam commented Nov 8, 2023

Fixes element-hq/element-web#26498 even when #3798 is present.

Listen for events being removed in the TimelineWIndow, and shorten the window if the removal makes it longer than the available events.


Here's what your changelog entry will look like:

🐛 Bug Fixes

@andybalaam andybalaam requested a review from a team as a code owner November 8, 2023 11:42
@andybalaam andybalaam requested review from dbkr and t3chguy November 8, 2023 11:42
src/timeline-window.ts Outdated Show resolved Hide resolved
@andybalaam andybalaam marked this pull request as draft November 8, 2023 12:21
@andybalaam andybalaam force-pushed the andybalaam/remove-event-on-window branch from 7114c3b to 6b40017 Compare November 8, 2023 16:26
@andybalaam andybalaam changed the title Provide a removeEvent method on TimelineWindow Shorten TimelineWindow when an event is removed Nov 8, 2023
@andybalaam andybalaam changed the title Shorten TimelineWindow when an event is removed Shorten TimelineWindow when an event is removed Nov 8, 2023
@@ -87,6 +92,7 @@ export class TimelineWindow {
opts: IOpts = {},
) {
this.windowLimit = opts.windowLimit || 1000;
timelineSet.room?.on(RoomEvent.Timeline, this.onTimelineEvent.bind(this));
Copy link
Member

Choose a reason for hiding this comment

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

This bind is superfluous, you're using an arrow function already.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, fixed in 7242aaa . I meant to declare it without the arrow function to match the surrounding code, but failed to update it after copy-pasting from somewhere else.

@andybalaam andybalaam requested a review from t3chguy November 9, 2023 13:36
@andybalaam andybalaam enabled auto-merge November 9, 2023 13:36
@andybalaam andybalaam added this pull request to the merge queue Nov 9, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 9, 2023
@andybalaam andybalaam added this pull request to the merge queue Nov 9, 2023
@andybalaam andybalaam removed this pull request from the merge queue due to a manual request Nov 9, 2023
@andybalaam andybalaam added this pull request to the merge queue Nov 9, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 9, 2023
@andybalaam andybalaam added this pull request to the merge queue Nov 9, 2023
Merged via the queue into develop with commit 5c160d0 Nov 9, 2023
19 checks passed
@andybalaam andybalaam deleted the andybalaam/remove-event-on-window branch November 9, 2023 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Messages stuck in "Sending your message..." state
2 participants