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

Align info EventTile and normal EventTile on IRC layout #10197

Merged
merged 13 commits into from
Mar 3, 2023
Merged

Align info EventTile and normal EventTile on IRC layout #10197

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
311440c
Reduce spacing values set with var(--right-padding) on mx_EventTile
luixxiul Feb 21, 2023
11e37fa
Remove spacing between profile name and trash bin
luixxiul Feb 21, 2023
323ddd3
For mx_RedactedBody use the inherited value for spacing, instead of t…
luixxiul Feb 21, 2023
31ca16e
Reflect the change to the test
luixxiul Feb 21, 2023
6d289b8
Add a test to check alignment
luixxiul Feb 23, 2023
34d1ad6
Merge branch 'develop' into fix-info-line-spacing-IRClayout
luixxiul Feb 23, 2023
5a8d1fa
Replace percySnapshot with percySnapshotElement
luixxiul Feb 23, 2023
ce32673
Improve the test
luixxiul Feb 23, 2023
d734ae5
Fix what to get
luixxiul Feb 23, 2023
ab00c5f
Add a test for emote
luixxiul Feb 23, 2023
4ec04d5
Check the button's text instead
luixxiul Feb 23, 2023
0041c01
Merge branch 'develop' into fix-info-line-spacing-IRClayout
luixxiul Feb 24, 2023
6b60dfc
Fix percyCSS for emote screenshot
luixxiul Feb 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions cypress/e2e/timeline/timeline.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,11 @@ describe("Timeline", () => {
// Check the event line has margin instead of inset property
// cf. _EventTile.pcss
// --EventTile_irc_line_info-margin-inline-start
// = calc(var(--name-width) + 10px + var(--icon-width))
// = 80 + 10 + 14 = 104px
// = calc(var(--name-width) + var(--icon-width) + 1 * var(--right-padding))
// = 80 + 14 + 5 = 99px

cy.get(".mx_EventTile[data-layout=irc].mx_EventTile_info:first-of-type .mx_EventTile_line")
.should("have.css", "margin-inline-start", "104px")
.should("have.css", "margin-inline-start", "99px")
.should("have.css", "inset-inline-start", "0px");

cy.get(".mx_Spinner").should("not.exist");
Expand Down
18 changes: 7 additions & 11 deletions res/css/views/rooms/_EventTile.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,11 @@ $left-gutter: 64px;
}

&[data-layout="irc"] {
--EventTile_irc_line_info-margin-inline-start: calc(var(--name-width) + 10px + var(--icon-width));
/* add --right-padding value of MessageTimestamp only */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

because mx_DisambiguatedProfile does not have right margin.

/* stylelint-disable-next-line declaration-colon-space-after */
--EventTile_irc_line_info-margin-inline-start: calc(
var(--name-width) + var(--icon-width) + 1 * var(--right-padding)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

1 * is set to clarify that right-padding value of just one element (MessageTimeStamp) is required to be added.

);

.mx_EventTile_msgOption {
.mx_ReadReceiptGroup {
Expand Down Expand Up @@ -483,20 +487,12 @@ $left-gutter: 64px;
}

&[data-layout="irc"] {
.mx_EventTile_line .mx_RedactedBody {
padding-left: 24px; /* 25px - 1px */

&::before {
left: var(--right-padding);
}
}
Comment on lines -486 to -492
Copy link
Contributor

@alunturner alunturner Feb 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

How come this is no longer required? I follow the rest of the changes but can't quite see how this can just be removed, perhaps I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, thanks. I'm checking why this was removed.

Copy link
Contributor Author

@luixxiul luixxiul Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Removing it cancels 5px adjustment for redacted messages with trash bin icons and aligns the lines with others.

First, this commit which removes padding-left lets the original value of .mx_RedactedBody, which has padding-left for the trash bin icon by default, be used. This commit has specified 24px in order to, I think, set 5px spacing to both right and left sides of the trash bin icon, whose width is 14px: (x-14px)/2=5px -> x=24px. I guess the comment /* 25px - 1px */ is not correct (I am not sure why I wrote so).

Second, this commit which removes left: var(--right-padding) aligns trash bin icons with other lines by canceling 5px adjustment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the detective work and the explanation


/* Apply only collapsed events block */
> .mx_EventTile_line {
/* 15 px of padding */
/* add --right-padding value of MessageTimestamp and avatar only */
/* stylelint-disable-next-line declaration-colon-space-after */
padding-left: calc(
var(--name-width) + var(--icon-width) + $MessageTimestamp_width + 3 * var(--right-padding)
var(--name-width) + var(--icon-width) + $MessageTimestamp_width + 2 * var(--right-padding)
);
}
}
Expand Down
3 changes: 2 additions & 1 deletion res/css/views/rooms/_IRCLayout.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ $irc-line-height: $font-18px;

.mx_EventTile_emote {
.mx_EventTile_avatar {
margin-left: calc(var(--name-width) + var(--icon-width) + var(--right-padding));
/* add --right-padding value of MessageTimestamp only */
margin-left: calc(var(--name-width) + var(--icon-width) + 1 * var(--right-padding));
}
}

Expand Down