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

Improve _IRCLayout.scss to remove space between the avatar and info text #8858

Closed
wants to merge 10 commits into from
Closed

Improve _IRCLayout.scss to remove space between the avatar and info text #8858

wants to merge 10 commits into from

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Jun 17, 2022 •
edited by github-actions bot
Loading

This PR organizes style rules on _IRCLayout.scss to clarify the structure.

Before After

type: task

Here's what your changelog entry will look like:

🐛 Bug Fixes

  • Improve _IRCLayout.scss to remove space between the avatar and info text (#8858). Contributed by @luixxiul.

@github-actions github-actions bot added Z-Community-PR Issue is solved by a community member's PR T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems labels Jun 17, 2022
@luixxiul luixxiul marked this pull request as ready for review June 17, 2022 10:08
@luixxiul luixxiul requested a review from a team as a code owner June 17, 2022 10:08
luixxiul
luixxiul commented Jun 18, 2022
View reviewed changes
res/css/views/rooms/_IRCLayout.scss Outdated
min-width: $MessageTimestamp_width;
}

.mx_EventTile[data-layout=irc] {
Copy link
Contributor Author

@luixxiul luixxiul Jun 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

:not() can increase the specificity of a rule, therefore including .mx_EventTile by .mx_IRCLayout seems to conflict with :not([data-layout=bubble]) and has not been quite enough without [data-layout=irc].

@luixxiul luixxiul mentioned this pull request Jun 24, 2022
Closed
@luixxiul luixxiul changed the title Improve _IRCLayout.scss to remove space between avatar and info text Remove space between avatar and info text, and inline start (left) margin from the reactions row on IRC layout Jun 24, 2022
luixxiul
luixxiul commented Jun 25, 2022
View reviewed changes
res/css/views/rooms/_IRCLayout.scss Outdated
.mx_ReactionsRow {
padding-left: 0;
padding-right: 0;
margin-inline: 0;
Copy link
Contributor Author

@luixxiul luixxiul Jun 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This works but it is redundant- removing the inline margin declaration from _EventTile.scss looks straightforward.

Addressed with #8891

@luixxiul luixxiul changed the title Remove space between avatar and info text, and inline start (left) margin from the reactions row on IRC layout Improve _IRCLayout.scss to remove space between the avatar and info text Jun 26, 2022
@luixxiul luixxiul marked this pull request as draft June 29, 2022 17:55
turt2live
turt2live reviewed Jun 29, 2022
View reviewed changes
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

clearing code review while draft

(@luixxiul please avoid converting things to drafts after they've been opened - if further work is needed, it's better to close and reopen the PR)

@luixxiul
Copy link
Contributor Author

Do you mind if I would ask the reason why converting to draft has to be avoided?

@turt2live
Copy link
Member

it ends up skewing our PR review latency statistics, and confuses the team trying to review the PR.

Opening a PR as a draft is fine, though once marked "ready for review" the assumption is that it's ready for review - it should not be converted back to draft as we cannot clear the code review request without manual intervention.

@luixxiul
Copy link
Contributor Author

luixxiul commented Jun 30, 2022 •
edited
Loading

I understood.

Is it not possible for admins to disable converting back something to draft? Otherwise a PR template should mention that at least, IMHO. Not everyone knows how PR review is conducted.

@turt2live
Copy link
Member

we're working on PR review guidance, and are fixing our contributing guidelines, though both are a bit slow (sorry).

luixxiul added 6 commits July 26, 2022 02:31
@luixxiul
Use a spacing variable
cbe4171 
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul
Sort by order properties
eedc52a 
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul
Remove a redundant declaration
5bbb8d3 
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul
Separate selectors
07e82ff 
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul
Merge style declarations - flex-shrink
8130210 
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul
Remove a redundant declaration about padding
d31d357 
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul
Copy link
Contributor Author

Closing as stale

@luixxiul luixxiul closed this Feb 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@turt2live turt2live turt2live left review comments

Assignees
No one assigned
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@luixxiul @turt2live