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

Make sending ContentLoaded optional for a widgetClient #4086

Merged

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Feb 28, 2024

For element call we want to have control over the sendContentLoaded action. As soon as we sent this action the widget will be shown.
However we need to adjust the theme first before we show the widget to not have an awkward flash of the wrong theme during the widget loading procedure.
So this should not be sent by the js-sdk and instead be sent by element call after the theme has been setup.
Signed-off-by: Timo K [email protected]

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

@toger5 toger5 requested a review from a team as a code owner February 28, 2024 15:26
@toger5 toger5 requested review from dbkr and richvdh February 28, 2024 15:26
@toger5 toger5 force-pushed the toger5/allow-disable-send-content-loaded-embedded-client branch from 5f32139 to 7dd89c8 Compare February 28, 2024 19:48
@toger5 toger5 changed the title Add sendContentLoaded option to widgetClient Add sending ContentLoaded optional for a widgetClient Feb 28, 2024
@toger5 toger5 changed the title Add sending ContentLoaded optional for a widgetClient Make sending ContentLoaded optional for a widgetClient Feb 28, 2024
@toger5 toger5 force-pushed the toger5/allow-disable-send-content-loaded-embedded-client branch from 7dd89c8 to 82bd422 Compare February 29, 2024 12:17
richvdh
richvdh previously requested changes Mar 1, 2024
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

There don't seem to be any tests for this - shouldn't there be?

src/matrix.ts Show resolved Hide resolved
src/embedded.ts Outdated Show resolved Hide resolved
Signed-off-by: Timo K <[email protected]>
Signed-off-by: Timo K <[email protected]>
src/embedded.ts Show resolved Hide resolved
@toger5 toger5 enabled auto-merge March 5, 2024 12:17
@toger5 toger5 requested a review from richvdh March 5, 2024 12:29
@toger5 toger5 dismissed richvdh’s stale review March 5, 2024 16:51

Documentation is written/corrected and I have added tests.

@toger5 toger5 added this pull request to the merge queue Mar 5, 2024
Signed-off-by: Timo K <[email protected]>
@toger5 toger5 removed this pull request from the merge queue due to a manual request Mar 5, 2024
@toger5 toger5 enabled auto-merge March 5, 2024 16:55
@toger5 toger5 added this pull request to the merge queue Mar 5, 2024
Merged via the queue into develop with commit 8c0736a Mar 5, 2024
23 checks passed
@toger5 toger5 deleted the toger5/allow-disable-send-content-loaded-embedded-client branch March 5, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants