Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Discard old sticker picker when the URL changes #2801

Merged
merged 1 commit into from
Mar 19, 2019

Conversation

turt2live
Copy link
Member

Fixes element-hq/element-web#7547


This is being PR'd as a community member:

Signed-off-by: Travis Ralston <travis@t2bot.io>

@turt2live turt2live requested a review from a team March 16, 2019 22:04
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Thanks! 😁 This looks reasonable to me.

@turt2live turt2live merged commit 9f6b550 into develop Mar 19, 2019
@turt2live turt2live deleted the travis/renew-widget branch March 19, 2019 14:50
turt2live added a commit that referenced this pull request Apr 2, 2019
Fixes element-hq/element-web#9354

#2801 introduced a change which tried to make sure that when the widget URL was changed that the picker would be re-mounted, however it accidentally introduced a regression. While it effectively did the task it wanted to, it failed to keep the previously-mounted sticker picker alive. This is because the Stickerpicker component is remounted when opened, and the _updateWidget function is called. This results in this.state not having the "current" widget, meaning a URL change is always detected when the component is remounted (room changes, open sticker picker). 

Instead of remounting always, we'll instead track which sticker picker widget is being used out of band. This therefore means that whenever the Stickerpicker component is mounted it doesn't create a whole new widget, and the existing (background) picker can be used. This also fixes the loading screen that people would see when opening the sticker picker after switching rooms, something which the persistent widget stuff is supposed to solve.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants