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

Tidy up IRCLayout.pcss #10189

Merged
merged 24 commits into from
Apr 23, 2023
Merged

Tidy up IRCLayout.pcss #10189

merged 24 commits into from
Apr 23, 2023

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Feb 20, 2023

This PR intends to tidy up IRCLayout.pcss to clarify its structure by grouping selectors which share the same declarations, sorting declarations based on order value, nesting, and removing a redundant declaration.

Since commits are split based on each change, reviewing per each commit should be easier than reviewing all of them at one time. Please note that there should not be an issue related to cascading which often happens when you move styles from one file to another, as this PR edits styles of just one file.

For a regression, most cases should be covered by the Cypress E2E tests.

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

type: task

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)

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

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>
Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
@github-actions github-actions bot added Z-Community-PR Issue is solved by a community member's PR T-Task Refactoring, enabling or disabling functionality, other engineering tasks labels Feb 20, 2023
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>
text-decoration: none;
min-width: $MessageTimestamp_width;
}
.mx_EventTile[data-layout="irc"] {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Protection against unintentional override by something in .mx_EventTile:not([data-layout="bubble"]) on _EventTile.pcss


display: flex;
flex-direction: row;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there is not an inherited value for the property, this declaration can safely be removed.

@luixxiul luixxiul marked this pull request as ready for review February 20, 2023 18:44
@luixxiul luixxiul requested a review from a team as a code owner February 20, 2023 18:44
@germain-gg germain-gg added the X-Needs-Percy Whether to run Percy screenshot tests in Merge Queue label Feb 21, 2023
@luixxiul
Copy link
Contributor Author

Please note that #10211 and #10197 have more extensive tests.

@SimonBrandner
Copy link
Contributor

Please note that #10211 and #10197 have more extensive tests.

Do I understand correctly that those PRs include tests that cover changes here?

@SimonBrandner
Copy link
Contributor

Can you re-request my review once this is ready to merge?

@luixxiul
Copy link
Contributor Author

luixxiul commented Mar 24, 2023

There are several changes to be approved on Percy tests for this PR. Please note that those changes are due to #10211, except one related to the focus issue.

CC @andybalaam

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
Copy link
Contributor

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

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

Looks sane

(apologies for the super late review; feel free to ping on Matrix next time :))

@SimonBrandner SimonBrandner added this pull request to the merge queue Apr 23, 2023
Merged via the queue into matrix-org:develop with commit ba79650 Apr 23, 2023
@luixxiul luixxiul deleted the IRCLayout branch April 23, 2023 08:12
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 X-Needs-Percy Whether to run Percy screenshot tests in Merge Queue 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.

3 participants