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

Remove mx_EventTile:not([data-layout="bubble"]) selector #9033

Merged
merged 12 commits into from
Apr 27, 2023
Merged

Remove mx_EventTile:not([data-layout="bubble"]) selector #9033

merged 12 commits into from
Apr 27, 2023

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Jul 9, 2022

Closes element-hq/element-web#24864

This PR intends to move declarations out of mx_EventTile:not([data-layout="bubble"]) to remove the selector and to make it easy to edit EventTile style rules without "hacks" to override the rules with increased specificity due to :not() pseudo class, merging the style rules on _EventBubbleTile.scss.

Because the rules inside mx_EventTile:not([data-layout="bubble"]) have been applied anywhere except mx_EventTile[data-layout="bubble"] even outside of the main timeline, such as on FilePanel. Therefore, this PR suggests to apply the declarations globally first and then cancel what has not been applied to bubble layout. This might look redundant, but this is conservative and safer.

The next task would be to clean up rules which have been added against increased specificity due to :not() pseudo class.

type: task

Signed-off-by: Suguru Hirahara luixxiul@users.noreply.github.com


This change is marked as an internal change (Task), so will not be included in the changelog.

@@ -403,7 +408,7 @@ $left-gutter: 64px;
}

&.mx_EventTile_continuation {
padding-top: 0px !important;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

merging style rules on _EventBubbleTile.scss

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
While mx_EventTile:not([data-layout="bubble"]) suggests that "clear: both" would not be applied to the bubble layout, the value "both" has been in fact luckily applied thanks to "mx_RoomView_MessageList li" on _RoomView.pcss, imported later than _EventTile.pcss.

Since "clear: both" is declared for mx_EventTile, this declaration is no longer needed here.

Though the value of that declaration of mx_EventTile on the timeline (including ThreadView) seems to have been overridden by "mx_RoomView_MessageList li" for all layouts, it should not be removed yet until E2E tests which check visual regressions are implemented, in order to minimize the risk of a regression outside of the timeline.

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
@luixxiul luixxiul changed the title Remove mx_EventTile:not([data-layout=bubble]) selector Remove mx_EventTile:not([data-layout="bubble"]) selector Apr 23, 2023
@luixxiul luixxiul marked this pull request as ready for review April 23, 2023 13:11
@luixxiul luixxiul requested a review from a team as a code owner April 23, 2023 13:11
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.

Changes here look good, but CI is unhappy for some reason. Will poke buttons to see if I can make it go.

@richvdh richvdh self-requested a review April 26, 2023 12:29
@richvdh richvdh added this pull request to the merge queue Apr 26, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 26, 2023
@richvdh richvdh added this pull request to the merge queue Apr 26, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 26, 2023
@luixxiul
Copy link
Contributor Author

The issue will be fixed by #10715.

@richvdh richvdh added this pull request to the merge queue Apr 27, 2023
Merged via the queue into matrix-org:develop with commit 569ef31 Apr 27, 2023
@luixxiul luixxiul deleted the positivism4 branch April 27, 2023 13:31
@richvdh
Copy link
Member

richvdh commented Apr 27, 2023

finally!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove :not([data-layout="bubble"]) pseudo class from mx_EventTile selector
2 participants