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

Theme loading without flickering #2233

Merged
merged 8 commits into from
Mar 13, 2024
Merged

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Feb 28, 2024

The theme did flicker because there is a default theme shown before any JS is loaded.
When the layoutEffect from the useTheme hook is reached the theme gets updated -> If its a different theme ("light", "dark") one can see a very short flicker.

This is particularly annoying in widget mode.

To work around this we only send the contentLoaded action once the theme is set.
This relies on the js sdk not sending this action automatically AND the react sdk needs to be configured with waitForIframe=false, so that it does not consider the widget to be ready once the IFrame is loaded.
Until the content_loadedaction is sent (which now is equivalent to: until EC is loaded), we will not show the widget. Instead the EW loading screen is shown.
This also saves us from the flickering: EW widget loading screen -> EC normal screen with header -> EC loading screen -> EC lobby.

This relies on:
Js-sdk PR that allows to disable sending the content loaded action right after the client is initialized but before the first react update (default behaviour of the js sdk):
matrix-org/matrix-js-sdk#4086

EW PR that sets waitForIframe=false and shows the widget loading screen until the receiving the content_loaded action:
matrix-org/matrix-react-sdk#12292
Signed-off-by: Timo K toger5@hotmail.de

The type failiours will be fixed once matrix-org/matrix-js-sdk#4086 is merged.

Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
@toger5 toger5 requested a review from a team as a code owner February 28, 2024 20:06
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
public/index.html Outdated Show resolved Hide resolved
@@ -13,7 +13,7 @@
</script>
</head>

<body class="cpd-theme-dark">
<body class="nodisplay">
Copy link
Member

Choose a reason for hiding this comment

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

Could we rename this class to something like no-theme so that it's more apparent that it's involved in the theme code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what about invisible-loading-theme (so its also clear that this class could be the reason why its not rendering anything.)

Copy link
Member

Choose a reason for hiding this comment

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

That's also fine by me

Signed-off-by: Timo K <toger5@hotmail.de>
@toger5 toger5 merged commit c932dd8 into livekit Mar 13, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants