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

Element Call: fix widget shown while its still loading (waitForIframeLoad=false) #12292

Merged
merged 9 commits into from
Mar 12, 2024

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Feb 27, 2024

Show loading spinner also if waitForIframeLoad = false.
Configure EC so it waits for the content loaded action.

This prohibits the error message:

Uncaught (in promise) Error: Improper sequence: not expecting ContentLoaded event if waitForIframeLoad is true (default=true)

!WARNING This breaks compatibility with the full mesh branch.
I would like to discuss here if/when we can do that.

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)

Configure EC so it waits for the content loaded action
!WARNING This breaks compatibility with the full mesh branch.
I would like to discuss here if/when we can do that.

Signed-off-by: Timo K <[email protected]>
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

So basically we are deciding that now is the time to drop support for any version of EC that doesn't support waitForIframeLoad: false? I think that seems fine: it has been quite some time. As you say, full mesh will no longer work but that may be fine.

And yeah, I'm not convinced about only passing the loading flag if waitForIframeLoad was true, I can't really make much sense of it and I can't think why we would have done it. I can't wait until this confusing flag is gone forever.

Could it get a type label and a more changelog-friendly title please?

@toger5 toger5 added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Feb 28, 2024
@toger5 toger5 changed the title Widget loading procedure patch Element Call: fix widget shown before done loading (waitForIframeLoad=false) Feb 28, 2024
@toger5 toger5 changed the title Element Call: fix widget shown before done loading (waitForIframeLoad=false) Element Call: fix widget shown before done loading (waitForIframeLoad=false) Feb 28, 2024
@toger5 toger5 changed the title Element Call: fix widget shown before done loading (waitForIframeLoad=false) Element Call: fix widget shown before its done loading (waitForIframeLoad=false) Feb 28, 2024
@toger5 toger5 changed the title Element Call: fix widget shown before its done loading (waitForIframeLoad=false) Element Call: fix widget shown while its still loading (waitForIframeLoad=false) Feb 28, 2024
@toger5
Copy link
Contributor Author

toger5 commented Feb 28, 2024

@dbkr updated the title and added T-Enhancement.

So basically we are deciding that now is the time to drop support for any version of EC that doesn't support waitForIframeLoad: false? I think that seems fine: it has been quite some time. As you say, full mesh will no longer work but that may be fine.

And yeah, I'm not convinced about only passing the loading flag if waitForIframeLoad was true, I can't really make much sense of it and I can't think why we would have done it. I can't wait until this confusing flag is gone forever.

Nice that you agree with those.

Signed-off-by: Timo K <[email protected]>
Signed-off-by: Timo K <[email protected]>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those snaps are now updated since we dont show the loading view for IFrames that have waitForIframeLoad=true (That are considered ready once the IFrame is ready.)

It would be possible to use the "preparing" event to show a spinner in the waitForIframeLoad=true cases. But to me it looks like this is how it was intended to be. (I dont see why waitForIFrame would have been used in getNewState otherwise)

@toger5 toger5 added this pull request to the merge queue Mar 12, 2024
Merged via the queue into develop with commit 396829e Mar 12, 2024
22 checks passed
@toger5 toger5 deleted the toger5/widget-loading-patch branch March 12, 2024 17:17
thoraj added a commit to verji/matrix-react-sdk that referenced this pull request Mar 13, 2024
* Upgrade dependency to [email protected]

* v3.94.0-rc.0

* Handle up/down as well as left/right for horizontal toolbars for improved a11y (matrix-org#12305)

Signed-off-by: Michael Telatynski <[email protected]>

* Remove references to internal js-sdk type `CryptoBackend` (matrix-org#12321)

* Remove references to internal js-sdk type `CryptoBackend`

* Use `Paramteters` to avoid `ts-ignore`

* Use `strong` element to semantically denote visually emphasised content (matrix-org#12320)

* Use `strong` element to semantically denote visually emphasised content

Signed-off-by: Michael Telatynski <[email protected]>

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Add comment

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

* Remove unused slider component (matrix-org#12303)

It is unused as of matrix-org#12246. I noticed this while working on matrix-org#12299.

* Update matrix-org (matrix-org#11966)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update all non-major dependencies (matrix-org#12322)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency stylelint-scss to v6.2.0 (matrix-org#12323)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency @vector-im/compound-web to v3.1.3 (matrix-org#12281)

* Update dependency @vector-im/compound-web to v3.1.3

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Fix TAC width due to compound update (matrix-org#12326)

---------

Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Florian Duros <[email protected]>

* Call guest access link creation to join calls as a non registered user via the EC SPA (matrix-org#12259)

* Add externall call link button if in public call room

Signed-off-by: Timo K <[email protected]>

* Allow configuring a spa homeserver url.

Signed-off-by: Timo K <[email protected]>

* temp

Signed-off-by: Timo K <[email protected]>

* remove homeserver url

Signed-off-by: Timo K <[email protected]>

* Add custom title to share dialog.
So that we can use it as a "share call" dialog.

Signed-off-by: Timo K <[email protected]>

* - rename config options
- only show link button if a guest url is provided
- share dialog custom Title
- rename call share labels

Signed-off-by: Timo K <[email protected]>

* rename to title_link

Signed-off-by: Timo K <[email protected]>

* add tests for ShareDialog

Signed-off-by: Timo K <[email protected]>

* add tests for share call button

Signed-off-by: Timo K <[email protected]>

* review

Signed-off-by: Timo K <[email protected]>

* remove comment

Signed-off-by: Timo K <[email protected]>

* Update src/components/views/dialogs/ShareDialog.tsx

Co-authored-by: David Baker <[email protected]>

---------

Signed-off-by: Timo K <[email protected]>
Co-authored-by: David Baker <[email protected]>

* Fix spotlight opening in TAC (matrix-org#12315)

* Fix spotlight opening in TAC

* Add tests

* Remove `RovingTabIndexProvider`

* Reset power selector on API failure to prevent state mismatch (matrix-org#12319)

* Reset power selector on API failure to prevent state mismatch

Signed-off-by: Michael Telatynski <[email protected]>

* Allow onChange to be sync or async

Signed-off-by: Michael Telatynski <[email protected]>

* Add unmounted check

Signed-off-by: Michael Telatynski <[email protected]>

* Improve coverage

Signed-off-by: Michael Telatynski <[email protected]>

* Iterate

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

* Use correct push rule to evaluate room-wide mentions (matrix-org#12318)

Signed-off-by: Michael Telatynski <[email protected]>

* Improve Forward Dialog a11y by switching to roving tab index interactions (matrix-org#12306)

* Improve Forward Dialog a11y by switching to roving tab index interactions

Signed-off-by: Michael Telatynski <[email protected]>

* Improve screen reader readout

Signed-off-by: Michael Telatynski <[email protected]>

* Improve screen reader readout

Signed-off-by: Michael Telatynski <[email protected]>

* Add tests

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

* [create-pull-request] automated change (matrix-org#12330)

Co-authored-by: github-merge-queue <[email protected]>

* Reuse media content/info types from the js-sdk (matrix-org#12308)

* TAC: Fix CSS & component typos (matrix-org#12333)

* Fix CSS & component typo

* Update snapshots

* Element Call: fix widget shown while its still loading (`waitForIframeLoad=false`) (matrix-org#12292)

* show loading spinner also if waitForIframeLoad = false
Configure EC so it waits for the content loaded action
!WARNING This breaks compatibility with the full mesh branch.
I would like to discuss here if/when we can do that.

Signed-off-by: Timo K <[email protected]>

* stop show loading screen on widget ready (instead of preparing)

Signed-off-by: Timo K <[email protected]>

* wait until widget loading is over before comparing screenshots

Signed-off-by: Timo K <[email protected]>

* fix waitForIFrame=true widgets

Signed-off-by: Timo K <[email protected]>

* test

Signed-off-by: Timo K <[email protected]>

* always start with loading true. + cleanup

Signed-off-by: Timo K <[email protected]>

* simplify loading

Signed-off-by: Timo K <[email protected]>

* update snapshots (start not in loading state for waitForIframe = true widgets)

Signed-off-by: Timo K <[email protected]>

---------

Signed-off-by: Timo K <[email protected]>

* Upgrade dependency to [email protected]

* v3.94.0

* Resetting package fields for development

* Reset matrix-js-sdk back to develop branch

---------

Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Timo K <[email protected]>
Co-authored-by: RiotRobot <[email protected]>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Robin <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Florian Duros <[email protected]>
Co-authored-by: Timo <[email protected]>
Co-authored-by: David Baker <[email protected]>
Co-authored-by: github-merge-queue <[email protected]>
thoraj added a commit to verji/matrix-react-sdk that referenced this pull request Mar 14, 2024
* Upgrade dependency to [email protected]

* v3.94.0-rc.0

* Handle up/down as well as left/right for horizontal toolbars for improved a11y (matrix-org#12305)

Signed-off-by: Michael Telatynski <[email protected]>

* Remove references to internal js-sdk type `CryptoBackend` (matrix-org#12321)

* Remove references to internal js-sdk type `CryptoBackend`

* Use `Paramteters` to avoid `ts-ignore`

* Use `strong` element to semantically denote visually emphasised content (matrix-org#12320)

* Use `strong` element to semantically denote visually emphasised content

Signed-off-by: Michael Telatynski <[email protected]>

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Add comment

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

* Remove unused slider component (matrix-org#12303)

It is unused as of matrix-org#12246. I noticed this while working on matrix-org#12299.

* Update matrix-org (matrix-org#11966)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update all non-major dependencies (matrix-org#12322)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency stylelint-scss to v6.2.0 (matrix-org#12323)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency @vector-im/compound-web to v3.1.3 (matrix-org#12281)

* Update dependency @vector-im/compound-web to v3.1.3

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Update snapshots

Signed-off-by: Michael Telatynski <[email protected]>

* Fix TAC width due to compound update (matrix-org#12326)

---------

Signed-off-by: Michael Telatynski <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Florian Duros <[email protected]>

* Call guest access link creation to join calls as a non registered user via the EC SPA (matrix-org#12259)

* Add externall call link button if in public call room

Signed-off-by: Timo K <[email protected]>

* Allow configuring a spa homeserver url.

Signed-off-by: Timo K <[email protected]>

* temp

Signed-off-by: Timo K <[email protected]>

* remove homeserver url

Signed-off-by: Timo K <[email protected]>

* Add custom title to share dialog.
So that we can use it as a "share call" dialog.

Signed-off-by: Timo K <[email protected]>

* - rename config options
- only show link button if a guest url is provided
- share dialog custom Title
- rename call share labels

Signed-off-by: Timo K <[email protected]>

* rename to title_link

Signed-off-by: Timo K <[email protected]>

* add tests for ShareDialog

Signed-off-by: Timo K <[email protected]>

* add tests for share call button

Signed-off-by: Timo K <[email protected]>

* review

Signed-off-by: Timo K <[email protected]>

* remove comment

Signed-off-by: Timo K <[email protected]>

* Update src/components/views/dialogs/ShareDialog.tsx

Co-authored-by: David Baker <[email protected]>

---------

Signed-off-by: Timo K <[email protected]>
Co-authored-by: David Baker <[email protected]>

* Fix spotlight opening in TAC (matrix-org#12315)

* Fix spotlight opening in TAC

* Add tests

* Remove `RovingTabIndexProvider`

* Reset power selector on API failure to prevent state mismatch (matrix-org#12319)

* Reset power selector on API failure to prevent state mismatch

Signed-off-by: Michael Telatynski <[email protected]>

* Allow onChange to be sync or async

Signed-off-by: Michael Telatynski <[email protected]>

* Add unmounted check

Signed-off-by: Michael Telatynski <[email protected]>

* Improve coverage

Signed-off-by: Michael Telatynski <[email protected]>

* Iterate

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

* Use correct push rule to evaluate room-wide mentions (matrix-org#12318)

Signed-off-by: Michael Telatynski <[email protected]>

* Improve Forward Dialog a11y by switching to roving tab index interactions (matrix-org#12306)

* Improve Forward Dialog a11y by switching to roving tab index interactions

Signed-off-by: Michael Telatynski <[email protected]>

* Improve screen reader readout

Signed-off-by: Michael Telatynski <[email protected]>

* Improve screen reader readout

Signed-off-by: Michael Telatynski <[email protected]>

* Add tests

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

* [create-pull-request] automated change (matrix-org#12330)

Co-authored-by: github-merge-queue <[email protected]>

* Reuse media content/info types from the js-sdk (matrix-org#12308)

* TAC: Fix CSS & component typos (matrix-org#12333)

* Fix CSS & component typo

* Update snapshots

* Element Call: fix widget shown while its still loading (`waitForIframeLoad=false`) (matrix-org#12292)

* show loading spinner also if waitForIframeLoad = false
Configure EC so it waits for the content loaded action
!WARNING This breaks compatibility with the full mesh branch.
I would like to discuss here if/when we can do that.

Signed-off-by: Timo K <[email protected]>

* stop show loading screen on widget ready (instead of preparing)

Signed-off-by: Timo K <[email protected]>

* wait until widget loading is over before comparing screenshots

Signed-off-by: Timo K <[email protected]>

* fix waitForIFrame=true widgets

Signed-off-by: Timo K <[email protected]>

* test

Signed-off-by: Timo K <[email protected]>

* always start with loading true. + cleanup

Signed-off-by: Timo K <[email protected]>

* simplify loading

Signed-off-by: Timo K <[email protected]>

* update snapshots (start not in loading state for waitForIframe = true widgets)

Signed-off-by: Timo K <[email protected]>

---------

Signed-off-by: Timo K <[email protected]>

* Upgrade dependency to [email protected]

* v3.94.0

* Resetting package fields for development

* Reset matrix-js-sdk back to develop branch

* Refine styles of menus, toasts, popovers, and modals (matrix-org#12332)

* Refine styles of menus, toasts, popovers, and modals

This is a reintroduction of matrix-org#12247, with the difference that modal styles have now been refreshed as well.

* Restore the fixed heights of some dialogs

* Fix formatting and flaky screenshot

* Make EC widget theme reactive - Update widget url when the theme changes (matrix-org#12295)

* update widget url when the theme changes

Signed-off-by: Timo K <[email protected]>

* quick "make it EC specific" workaround proposal.

Signed-off-by: Timo K <[email protected]>

* use `matches`

Signed-off-by: Timo K <[email protected]>

* test coverage

Signed-off-by: Timo K <[email protected]>

* more test coverage

Signed-off-by: Timo K <[email protected]>

* fix jest

Signed-off-by: Timo K <[email protected]>

* add tests for theme changes

Signed-off-by: Timo K <[email protected]>

* update snapshots

Signed-off-by: Timo K <[email protected]>

* test for theme update with non ec widget

Signed-off-by: Timo K <[email protected]>

* add dark custom theme widget url

Signed-off-by: Timo K <[email protected]>

* trigger conditions for theme cleanup

Signed-off-by: Timo K <[email protected]>

* update tests using testId

Signed-off-by: Timo K <[email protected]>

* use typed event emitter for theme watcher

Signed-off-by: Timo K <[email protected]>

* simplify condition

Signed-off-by: Timo K <[email protected]>

---------

Signed-off-by: Timo K <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Timo K <[email protected]>
Co-authored-by: RiotRobot <[email protected]>
Co-authored-by: Michael Telatynski <[email protected]>
Co-authored-by: Richard van der Hoff <[email protected]>
Co-authored-by: Robin <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Florian Duros <[email protected]>
Co-authored-by: Timo <[email protected]>
Co-authored-by: David Baker <[email protected]>
Co-authored-by: github-merge-queue <[email protected]>
andybalaam pushed a commit that referenced this pull request Mar 18, 2024
…eLoad=false`) (#12292)

* show loading spinner also if waitForIframeLoad = false
Configure EC so it waits for the content loaded action
!WARNING This breaks compatibility with the full mesh branch.
I would like to discuss here if/when we can do that.

Signed-off-by: Timo K <[email protected]>

* stop show loading screen on widget ready (instead of preparing)

Signed-off-by: Timo K <[email protected]>

* wait until widget loading is over before comparing screenshots

Signed-off-by: Timo K <[email protected]>

* fix waitForIFrame=true widgets

Signed-off-by: Timo K <[email protected]>

* test

Signed-off-by: Timo K <[email protected]>

* always start with loading true. + cleanup

Signed-off-by: Timo K <[email protected]>

* simplify loading

Signed-off-by: Timo K <[email protected]>

* update snapshots (start not in loading state for waitForIframe = true widgets)

Signed-off-by: Timo K <[email protected]>

---------

Signed-off-by: Timo K <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants