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

Fix regression around CSS stacking contexts and PIP widgets #12094

Merged
merged 16 commits into from
Jan 8, 2024

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Dec 29, 2023

Fixes element-hq/element-web#26805

Regressed by #12052

It added a rule #matrixchat { contain: strict; } which changed how the layout engine works.
It enabled CSS stacking contexts, which z-index cannot cross, its like a 2nd order z-index, the buttons were in the wrong stacking context for where they wanted to be drawn, so the PR moves them to the correct context (by moving it into the PersistentElement as that's a React Portal which renders outside of #matrixchat)


Here's what your changelog entry will look like:

🐛 Bug Fixes

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
… to continue functioning

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@toger5
Copy link
Contributor

toger5 commented Dec 29, 2023

Thank you so much!
There are issues with the styling:

  • The border radius of 8px is missing: this also missing in the css right now but will be added back in https://github.com/matrix-org/matrix-react-sdk/pull/12057/files
    This also is not working witht he new drawing anymore (I tried adding the radius in the devtools but the IFrame did not accept :border-radius: 8px;overflow: hidden; The border-radius work when adding a 0.989 opacity to the IFrame.
  • The hangup and back button and footer/header gradients are too dark

… t3chguy/fix/26805

# Conflicts:
#	res/css/_common.pcss
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Copy link
Member Author

t3chguy commented Jan 2, 2024

The hangup and back button and footer/header gradients are too dark

I cannot reproduce this.

Before
image

After
image

Can reproduce the border radius issue though so will fix that.

After^2
image

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@toger5
Copy link
Contributor

toger5 commented Jan 2, 2024

This is what it looks for me:
Screenshot 2024-01-02 at 12 05 41
On firefox. Looking through the css it seems like there is no explicit color css property on the hangup icon and the title text.

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@t3chguy
Copy link
Member Author

t3chguy commented Jan 2, 2024

@toger5 cannot reproduce.

Before
image

After
image

Actually comparing the screenshots looks like app.element.io is darker?

@toger5
Copy link
Contributor

toger5 commented Jan 2, 2024

This is what it looks for me: Screenshot 2024-01-02 at 12 05 41 On firefox. Looking through the css it seems like there is no explicit color css property on the hangup icon and the title text.

I also tried on chrome where it looks the same as on FF.
Are you trying with you local build or with netlify? I am on trying with the netlify build. Maybe some dependencies are different.

@t3chguy
Copy link
Member Author

t3chguy commented Jan 2, 2024

image

I was trying on a local build but the above is on the Netlify.

@t3chguy t3chguy marked this pull request as ready for review January 2, 2024 12:23
@t3chguy t3chguy requested a review from a team as a code owner January 2, 2024 12:23
@t3chguy t3chguy added the X-Release-Blocker This affects the current release cycle and must be solved for a release to happen label Jan 2, 2024
@fkwp
Copy link
Contributor

fkwp commented Jan 3, 2024

image

same issue for me on safari using the netlify build

@richvdh
Copy link
Member

richvdh commented Jan 3, 2024

If this was a regression, could you link the PR that caused it, to help understand the fix? Or generally add some detail to the issue about how the problem came to be?

@toger5
Copy link
Contributor

toger5 commented Jan 3, 2024

If this was a regression, could you link the PR that caused it, to help understand the fix? Or generally add some detail to the issue about how the problem came to be?

The problem occured because of using the strict contained mode as described in the issue: element-hq/element-web#26805 (comment).
The regression is: no interaction and wrong styling with pip elements.

@fkwp and me are having issues with the fix however that there are styling issues: (icon and text color is too dark). But @t3chguy cannot reproduce those.

@richvdh
Copy link
Member

richvdh commented Jan 4, 2024

The problem occured because of using the strict contained mode as described in the issue: element-hq/element-web#26805 (comment).

I'm afraid I still don't really follow what changed.

@fkwp and me are having issues with the fix however that there are styling issues: (icon and text color is too dark). But @t3chguy cannot reproduce those.

Sounds like this is still in development, so moving this to draft for now.

@richvdh richvdh marked this pull request as draft January 4, 2024 10:49
@t3chguy
Copy link
Member Author

t3chguy commented Jan 4, 2024

image

Looks like its specific to light theme.

image

Moved the colour to fix.

image

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Copy link
Contributor

@toger5 toger5 left a comment

Choose a reason for hiding this comment

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

Great! Thanks for fixing the light theme.

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.

I have no real idea what's going on here, I'm afraid. It looks vaguely plausible. Since I don't understand the nuclear reactor, I have some questions about the colour you have decided to paint the bikeshed.

Comment on lines +59 to +62
#mx_ContextualMenu_Container,
#mx_PersistedElement_container,
#mx_Dialog_Container,
#mx_Dialog_StaticContainer {
Copy link
Member

Choose a reason for hiding this comment

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

why are these changing?

Copy link
Member Author

Choose a reason for hiding this comment

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

So that they layer correctly relative to the PIP widget and tooltips and each other by having their own CSS stacking contexts

Comment on lines +96 to +97
// An element to render after the iframe as an overlay
overlay?: ReactNode;
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't it make more sense for this to be passed as children, rather than a regular prop?

Copy link
Member Author

Choose a reason for hiding this comment

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

Children have different semantics of typically always being included, this only includes it when the iframe is shown, not when the loader or permissions prompt are shown

Copy link
Member

Choose a reason for hiding this comment

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

Children have different semantics of typically always being included

I'm not entirely convinced about that. And if it's true, doesn't the same logic apply to PersistentApp?

But I don't feel strongly about it.

Comment on lines +328 to +330
&.mx_AppTileBody--call.mx_AppTileBody--mini {
border-radius: 8px;
}
Copy link
Member

Choose a reason for hiding this comment

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

why is this now needed (as well as the border-radius on .mx_WidgetPip_overlay ?

(I have no real idea what a &.mx_AppTileBody--call.mx_AppTileBody--mini is, I'm afraid)

Copy link
Contributor

@toger5 toger5 Jan 5, 2024

Choose a reason for hiding this comment

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

Before the widget was always rendered the default app container with rounded borders.
This is a concious design decision to underline that it is not part of Eement web but potentially a thrid party app running as a widget. For calls we want to special case this and show the widget as if its part of Element Web.
So mx_AppTileBody--call has border-radius: 0px;.
For the PiP call view rounded corners are alwasy required. So we set it back to 8px.

Old:
Screenshot 2024-01-05 at 21 49 58
Now:
Screenshot 2024-01-05 at 21 56 58
Issue caused by new design
Screenshot 2024-01-05 at 21 53 16
With:

    &.mx_AppTileBody--call.mx_AppTileBody--mini {
        border-radius: 8px;
    }
Screenshot 2024-01-05 at 21 53 49

Copy link
Member

@richvdh richvdh Jan 8, 2024

Choose a reason for hiding this comment

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

Before the widget was always rendered the default app container with rounded borders.

I don't follow. There don't seem to be enough words in this sentence?

Eement

Element

alwasy

always

src/components/structures/PipContainer.tsx Show resolved Hide resolved
test/setupTests.ts Outdated Show resolved Hide resolved
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
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.

seems plausible to me

@t3chguy t3chguy enabled auto-merge January 8, 2024 10:49
@t3chguy t3chguy added this pull request to the merge queue Jan 8, 2024
Merged via the queue into develop with commit 57da29d Jan 8, 2024
24 checks passed
@t3chguy t3chguy deleted the t3chguy/fix/26805 branch January 8, 2024 12:36
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 25, 2024
Changes in [1.11.55](https://github.com/element-hq/element-web/releases/tag/v1.11.55) (2024-01-19)
==================================================================================================


## ✨ Features

* Broaden support for matrix spec versions ([#12159](matrix-org/matrix-react-sdk#12159)). Contributed by @RiotRobot.

## 🐛 Bug Fixes

* Fixed shield alignment on message Input ([#12155](matrix-org/matrix-react-sdk#12155)). Contributed by @RiotRobot.


Changes in [1.11.54](https://github.com/element-hq/element-web/releases/tag/v1.11.54) (2024-01-16)
==================================================================================================
## 🐛 Bug Fixes

* Fix CSS stacking context order determinism ([#26840](element-hq/element-web#26840)). Contributed by @t3chguy.

## ✨ Features

* Accessibility improvements around aria-labels and tooltips ([#12062](matrix-org/matrix-react-sdk#12062)). Contributed by @t3chguy.
* Add RoomKnocksBar to RoomHeader ([#12077](matrix-org/matrix-react-sdk#12077)). Contributed by @charlynguyen.
* Adjust tooltip side for DecoratedRoomAvatar to not obscure room name ([#12079](matrix-org/matrix-react-sdk#12079)). Contributed by @t3chguy.
* Iterate landmarks around the app in order to improve a11y ([#12064](matrix-org/matrix-react-sdk#12064)). Contributed by @t3chguy.
* Update element call embedding UI ([#12056](matrix-org/matrix-react-sdk#12056)). Contributed by @toger5.
* Use Compound tooltips instead of homegrown in TextWithTooltip \& InfoTooltip ([#12052](matrix-org/matrix-react-sdk#12052)). Contributed by @t3chguy.

## 🐛 Bug Fixes

* Fix regression around CSS stacking contexts and PIP widgets ([#12094](matrix-org/matrix-react-sdk#12094)). Contributed by @t3chguy.
* Fix Identity Server terms accepting not working as expected ([#12109](matrix-org/matrix-react-sdk#12109)). Contributed by @t3chguy.
* fix: microphone and camera dropdown doesn't work In legacy call ([#12105](matrix-org/matrix-react-sdk#12105)). Contributed by @muratersin.
* Revert "Set up key backup using non-deprecated APIs (#12005)" ([#12102](matrix-org/matrix-react-sdk#12102)). Contributed by @BillCarsonFr.
* Fix regression around read receipt animation from refs changes ([#12100](matrix-org/matrix-react-sdk#12100)). Contributed by @t3chguy.
* Added meaning full error message based on platform ([#12074](matrix-org/matrix-react-sdk#12074)). Contributed by @Pankaj-SinghR.
* Fix editing event from search room view ([#11992](matrix-org/matrix-react-sdk#11992)). Contributed by @t3chguy.
* Fix timeline position when moving to a room and coming back ([#12055](matrix-org/matrix-react-sdk#12055)). Contributed by @florianduros.
* Fix threaded reply playwright tests ([#12070](matrix-org/matrix-react-sdk#12070)). Contributed by @dbkr.
* Element-R: fix repeated requests to enter 4S key during cross-signing reset ([#12059](matrix-org/matrix-react-sdk#12059)). Contributed by @richvdh.
* Fix position of thumbnail in room timeline ([#12016](matrix-org/matrix-react-sdk#12016)). Contributed by @anoopw3bdev.


Changes in [1.11.53](https://github.com/element-hq/element-web/releases/tag/v1.11.53) (2024-01-04)
==================================================================================================

## 🐛 Bug Fixes

* Fix a fresh login creating a new key backup ([#12106](matrix-org/matrix-react-sdk#12106)).


Changes in [1.11.52](https://github.com/element-hq/element-web/releases/tag/v1.11.52) (2023-12-19)
==================================================================================================


## ✨ Features

* Keep more recent rageshake logs ([#12003](matrix-org/matrix-react-sdk#12003)). Contributed by @richvdh.

## 🐛 Bug Fixes

* Fix bug which prevented correct clean up of rageshake store ([#12002](matrix-org/matrix-react-sdk#12002)). Contributed by @richvdh.
* Set up key backup using non-deprecated APIs ([#12005](matrix-org/matrix-react-sdk#12005)). Contributed by @andybalaam.
* Fix notifications appearing for old events ([#3946](matrix-org/matrix-js-sdk#3946)). Contributed by @dbkr.
* Prevent phantom notifications from events not in a room's timeline ([#3942](matrix-org/matrix-js-sdk#3942)). Contributed by @dbkr.


Changes in [1.11.51](https://github.com/vector-im/element-web/releases/tag/v1.11.51) (2023-12-05)
=================================================================================================
## ✨ Features

* Improve debian package and docs ([#26618](element-hq/element-web#26618)). Contributed by @t3chguy.

## 🦖 Deprecations

* Remove Quote from MessageContextMenu as it is unsupported by WYSIWYG ([#11914](matrix-org/matrix-react-sdk#11914)). Contributed by @t3chguy.

## ✨ Features

* Always allow call.member events on new rooms ([#11948](matrix-org/matrix-react-sdk#11948)). Contributed by @toger5.
* Right panel: view third party invite info without clearing history ([#11934](matrix-org/matrix-react-sdk#11934)). Contributed by @kerryarchibald.
* Allow switching to system emoji font ([#11925](matrix-org/matrix-react-sdk#11925)). Contributed by @t3chguy.
* Update open in other tab message ([#11916](matrix-org/matrix-react-sdk#11916)). Contributed by @weeman1337.
* Add menu for legacy and element call in 1:1 rooms ([#11910](matrix-org/matrix-react-sdk#11910)). Contributed by @toger5.
* Add ringing for matrixRTC ([#11870](matrix-org/matrix-react-sdk#11870)). Contributed by @toger5.

## 🐛 Bug Fixes

* Keep device language when it has been previosuly set, after a successful delegated authentication flow that clears localStorage ([#11902](matrix-org/matrix-react-sdk#11902)). Contributed by @mgcm.
* Fix misunderstanding of functional members ([#11918](matrix-org/matrix-react-sdk#11918)). Contributed by @toger5.
* Fix: Video Room Chat Header Button Removed ([#11911](matrix-org/matrix-react-sdk#11911)). Contributed by @kerryarchibald.
* Fix "not attempting encryption" warning ([#11899](matrix-org/matrix-react-sdk#11899)). Contributed by @richvdh.


Changes in [1.11.50](https://github.com/vector-im/element-web/releases/tag/v1.11.50) (2023-11-21)
=================================================================================================

## ✨ Features

* Ship element-web as a debian package ([#26533](element-hq/element-web#26533)). Contributed by @t3chguy.
* Update room summary card header ([#11823](matrix-org/matrix-react-sdk#11823)). Contributed by @germain-gg.
* Add feature flag for disabling encryption in Element Call ([#11837](matrix-org/matrix-react-sdk#11837)). Contributed by @toger5.
* Adapt the rendering of extra icons in the room header ([#11835](matrix-org/matrix-react-sdk#11835)). Contributed by @charlynguyen.
* Implement new unreachable state and fix broken string ref  ([#11748](matrix-org/matrix-react-sdk#11748)). Contributed by @MidhunSureshR.
* Allow adding extra icons to the room header ([#11799](matrix-org/matrix-react-sdk#11799)). Contributed by @charlynguyen.

## 🐛 Bug Fixes

* Room header: do not collapse avatar or facepile ([#11866](matrix-org/matrix-react-sdk#11866)). Contributed by @kerryarchibald.
* New right panel: fix button alignment in memberlist ([#11861](matrix-org/matrix-react-sdk#11861)). Contributed by @kerryarchibald.
* Use the correct video call icon variant ([#11859](matrix-org/matrix-react-sdk#11859)). Contributed by @robintown.
* fix broken warning icon ([#11862](matrix-org/matrix-react-sdk#11862)). Contributed by @ara4n.
* Fix rightpanel hiding scrollbar ([#11831](matrix-org/matrix-react-sdk#11831)). Contributed by @kerryarchibald.
* Switch to updating presence via /sync calls instead of PUT /presence ([#11824](matrix-org/matrix-react-sdk#11824)). Contributed by @t3chguy.

Changes in [1.11.49](https://github.com/vector-im/element-web/releases/tag/v1.11.49) (2023-11-13)
=================================================================================================

## ✨ Features
 * Ship element-web as a debian package ([\#26533](element-hq/element-web#26533)). Fixes #2777.

## 🐛 Bug Fixes
 * Ensure `setUserCreator` is called when a store is assigned ([\#3867](matrix-org/matrix-js-sdk#3867)). Fixes element-hq/element-web#26520. Contributed by @MidhunSureshR.

Changes in [1.11.48](https://github.com/vector-im/element-web/releases/tag/v1.11.48) (2023-11-07)
=================================================================================================

## ✨ Features
 * Correctly fill window.matrixChat even when a Wrapper module is active ([\#26395](element-hq/element-web#26395)). Contributed by @dhenneke.
 * Knock on a ask-to-join room if a module wants to join the room when navigating to a room ([\#11787](matrix-org/matrix-react-sdk#11787)). Contributed by @dhenneke.
 * Element-R:  Include crypto info in sentry ([\#11798](matrix-org/matrix-react-sdk#11798)). Contributed by @florianduros.
 * Element-R:  Include crypto info in rageshake ([\#11797](matrix-org/matrix-react-sdk#11797)). Contributed by @florianduros.
 * Element-R: Add current version of the rust-sdk and vodozemac ([\#11785](matrix-org/matrix-react-sdk#11785)). Contributed by @florianduros.
 * Fix unfederated invite dialog ([\#9618](matrix-org/matrix-react-sdk#9618)). Fixes element-hq/element-meta#1466 and #22102. Contributed by @owi92.
 * New right panel visual language ([\#11664](matrix-org/matrix-react-sdk#11664)).
 * OIDC: add friendly errors ([\#11184](matrix-org/matrix-react-sdk#11184)). Fixes #25665. Contributed by @kerryarchibald.

## 🐛 Bug Fixes
 * Fix rightpanel hiding scrollbar ([\#11831](matrix-org/matrix-react-sdk#11831)). Contributed by @kerryarchibald.
 * Fix multi-tab session lock on Firefox not being cleared ([\#11800](matrix-org/matrix-react-sdk#11800)). Fixes #26165. Contributed by @ManuelHu.
 * Deserialise spoilers back into slash command form ([\#11805](matrix-org/matrix-react-sdk#11805)). Fixes #26344.
 * Fix Incorrect message scaling for verification request ([\#11793](matrix-org/matrix-react-sdk#11793)). Fixes #24304. Contributed by @capGoblin.
 * Fix: Unable to restore a soft-logged-out session established via SSO ([\#11794](matrix-org/matrix-react-sdk#11794)). Fixes #25957. Contributed by @kerryarchibald.
 * Use configurable github issue links more consistently ([\#11796](matrix-org/matrix-react-sdk#11796)).
 * Fix io.element.late_event received_ts vs received_at ([\#11789](matrix-org/matrix-react-sdk#11789)).
 * Make invitation dialog scrollable when infos are too long ([\#11753](matrix-org/matrix-react-sdk#11753)). Contributed by @nurjinjafar.
 * Fix spoiler text-align ([\#11790](matrix-org/matrix-react-sdk#11790)). Contributed by @ajbura.
 * Fix: Right panel keeps showing chat when unmaximizing widget.  ([\#11697](matrix-org/matrix-react-sdk#11697)). Fixes #26265. Contributed by @manancodes.
 * Fix margin of invite to room button ([\#11780](matrix-org/matrix-react-sdk#11780)). Fixes #26410.
 * Update base64 import ([\#11784](matrix-org/matrix-react-sdk#11784)).
 * Set max size for Element logo in search warning ([\#11779](matrix-org/matrix-react-sdk#11779)). Fixes #26408.
 * Fix: emoji size in room header topic, remove obsolete emoji style ([\#11757](matrix-org/matrix-react-sdk#11757)). Fixes #26326. Contributed by @kerryarchibald.
 * Fix: Bubble layout design is broken ([\#11763](matrix-org/matrix-react-sdk#11763)). Fixes #25818. Contributed by @manancodes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems X-Release-Blocker This affects the current release cycle and must be solved for a release to happen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PiP-PersistedElement rendered in front of Pip-buttons (Hiding call hangup and GoTo room buttons)
4 participants