Removes the override on the Bubble Container

[DantrazTrev]

Fixes element-hq/element-web/issues/17016

The .mx_EventTile_bubbleContainer's .mx_EventTile_line overrides padding to zero

matrix-react-sdk/res/css/views/rooms/_EventTile.scss

Line 127 in 8dbcc85

padding: 0 !important;

Which affects the m.room.verification if the sender is not the user

When the sender is defined as the user it renders correctly

However, the padding overriding could be removed and it provides the correct results for either case

Signed-off-by : Ayush Pratap Singh ayushpratap16@gmail.com

[t3chguy]

This looks to regress the centring which the line is there to do.

Before:

After:

The bubble tile is no longer centred.

[DantrazTrev]

@t3chguy I think m.key.verification.___ and few more events weren't supposed to be rendered if the sender is not defined as the user but allowing hidden event adds the issue of them falling back to the default mx_EventTile_line. I'll add another CSS class to make sure they're rendered evenly.

[jryans]

Sounds like further changes are needed.

[DantrazTrev]

Added the hidden class for the mx_EventTile_bubbleContainer for the events so they render correctly

[jryans]

@DantrazTrev Could you merge develop into your branch? It looks like there's a conflict to resolve, and getting the latest code will also help the tests pass as well.

[DantrazTrev]

@DantrazTrev Could you merge develop into your branch? It looks like there's a conflict to resolve, and getting the latest code will also help the tests pass as well.

Done ^-^

[jryans]

Hmm, rather than overriding bubble styling, I wonder if instead we should resolve this by disabling bubble mode when we're using the hidden event handler. In more detail, perhaps this block:

matrix-react-sdk/src/components/views/rooms/EventTile.tsx

Lines 841 to 845 in 12a9ce1

	if (SettingsStore.getValue("showHiddenEventsInTimeline") && !haveTileForEvent(this.props.mxEvent)) {
	tileHandler = "messages.ViewSourceEvent";
	// Reuse info message avatar and sender profile styling
	isInfoMessage = true;
	}

should reset isBubbleMessage to false...?

[DantrazTrev]

Hmm, rather than overriding bubble styling, I wonder if instead we should resolve this by disabling bubble mode when we're using the hidden event handler. In more detail, perhaps this block:

matrix-react-sdk/src/components/views/rooms/EventTile.tsx

Lines 841 to 845 in 12a9ce1

	if (SettingsStore.getValue("showHiddenEventsInTimeline") && !haveTileForEvent(this.props.mxEvent)) {
	tileHandler = "messages.ViewSourceEvent";
	// Reuse info message avatar and sender profile styling
	isInfoMessage = true;
	}

should reset isBubbleMessage to false...?

Not really sure about it, considering it is the fallback to a tile that is causing the error. Disabling it would totally remove the tile and we might need to design a new one. Just disabling it would also not have really good consequences for all hidden events

The messages that are causing the error don't get rendered at all in the timeline as they're a part of pair of verification events, however, explicitly sending them through the dev tools primarily causes this issue. Maybe we should not allow them to be rendered from there?

[jryans]

Not really sure about it, considering it is the fallback to a tile that is causing the error. Disabling it would totally remove the tile and we might need to design a new one. Just disabling it would also not have really good consequences for all hidden events

The messages that are causing the error don't get rendered at all in the timeline as they're a part of pair of verification events, however, explicitly sending them through the dev tools primarily causes this issue. Maybe we should not allow them to be rendered from there?

Sorry if I am being dense, but what do you mean "tile that is causing the error"? I thought this PR was only about alignment of certain tiles...? What is "the error"?

In the block I quoted above:

if (SettingsStore.getValue("showHiddenEventsInTimeline") && !haveTileForEvent(this.props.mxEvent)) { 

...there is no "error" when we fail to find a tile, if that's what you mean...? We just don't have a tile for some custom event. If hidden events are shown (which is intended for developers only), we go into this branch and show the hidden event view. I am suggesting we could potentially resolve the alignment by adding a line to that block which sets isBubbleMessage to false for such a case.

Are you saying that you tried that, and it causes the call tiles to soft crash with "can't load the message" errors? That seems very surprising to me, so I must be misunderstanding... I was expecting isBubbleMessage to mainly affect alignment and appearance only, since it controls whether some CSS classes are set.

[DantrazTrev]

Not really sure about it, considering it is the fallback to a tile that is causing the error. Disabling it would totally remove the tile and we might need to design a new one. Just disabling it would also not have really good consequences for all hidden events

The messages that are causing the error don't get rendered at all in the timeline as they're a part of pair of verification events, however, explicitly sending them through the dev tools primarily causes this issue. Maybe we should not allow them to be rendered from there?

Sorry if I am being dense, but what do you mean "tile that is causing the error"? I thought this PR was only about alignment of certain tiles...? What is "the error"?

In the block I quoted above:

if (SettingsStore.getValue("showHiddenEventsInTimeline") && !haveTileForEvent(this.props.mxEvent)) { 

...there is no "error" when we fail to find a tile, if that's what you mean...? We just don't have a tile for some custom event. If hidden events are shown (which is intended for developers only), we go into this branch and show the hidden event view. I am suggesting we could potentially resolve the alignment by adding a line to that block which sets isBubbleMessage to false for such a case.

Are you saying that you tried that, and it causes the call tiles to soft crash with "can't load the message" errors? That seems very surprising to me, so I must be misunderstanding... I was expecting isBubbleMessage to mainly affect alignment and appearance only, since it controls whether some CSS classes are set.

I apologize @jryans , I seem to have forgotten what I did back then that caused that error ( Potentially could have disabled something else along with that , that might have caused the error ) But now that I rechecked yes it does fix the issue alignment.

And yes you're correct that disabling isBubbleMessage for fallback tiles in the block fixes the issue and renders them as user sent events .

[jryans]

Great, thanks for working on this!