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 all 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
114 changes: 111 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");

// Exclude timestamp from snapshot
Expand All @@ -194,6 +195,113 @@ describe("Timeline", () => {
cy.checkA11y();
});

it("should align generic event list summary with messages and emote on IRC layout", () => {
// This test aims to check:
// 1. Alignment of collapsed GELS (generic event list summary) and messages
// 2. Alignment of expanded GELS and messages
// 3. Alignment of expanded GELS and placeholder of deleted message
// 4. Alignment of expanded GELS, placeholder of deleted message, and emote

// Exclude timestamp from snapshot of mx_MainSplit
const percyCSS = ".mx_MainSplit .mx_MessageTimestamp { visibility: hidden !important; }";
Copy link
Contributor Author

@luixxiul luixxiul Feb 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This should hide timestamps of not only an info event line but also an emote (and a normal event line for messages) for https://percy.io/dfde73bd/matrix-react-sdk/builds/25434742/changed/1418297380?browser=edge&browser_ids=18%2C33%2C34%2C35&subcategories=approved&viewLayout=overlay&viewMode=new&width=1920&widths=400%2C800%2C1024%2C1920.


cy.visit("/#/room/" + roomId);
cy.setSettingValue("layout", null, SettingLevel.DEVICE, Layout.IRC);

// Wait until configuration is finished
cy.contains(
".mx_RoomView_body .mx_GenericEventListSummary .mx_GenericEventListSummary_summary",
"created and configured the room.",
).should("exist");

// Send messages
cy.get(".mx_RoomView_body .mx_BasicMessageComposer_input").type("Hello Mr. Bot{enter}");
cy.get(".mx_RoomView_body .mx_BasicMessageComposer_input").type("Hello again, Mr. Bot{enter}");
// Make sure the second message was sent
cy.get(".mx_RoomView_MessageList > .mx_EventTile_last .mx_EventTile_receiptSent").should("be.visible");

// 1. Alignment of collapsed GELS (generic event list summary) and messages
// Check inline start spacing of collapsed GELS
// See: _EventTile.pcss
// .mx_GenericEventListSummary[data-layout="irc"] > .mx_EventTile_line
// = var(--name-width) + var(--icon-width) + $MessageTimestamp_width + 2 * var(--right-padding)
// = 80 + 14 + 46 + 2 * 5
// = 150px
cy.get(".mx_GenericEventListSummary[data-layout=irc] > .mx_EventTile_line").should(
"have.css",
"padding-inline-start",
"150px",
);
// Check width and spacing values of elements in .mx_EventTile, which should be equal to 150px
// --right-padding should be applied
cy.get(".mx_EventTile > *").should("have.css", "margin-right", "5px");
// --name-width width zero inline end margin should be applied
cy.get(".mx_EventTile .mx_DisambiguatedProfile")
.should("have.css", "width", "80px")
.should("have.css", "margin-inline-end", "0px");
// --icon-width should be applied
cy.get(".mx_EventTile .mx_EventTile_avatar > .mx_BaseAvatar").should("have.css", "width", "14px");
// $MessageTimestamp_width should be applied
cy.get(".mx_EventTile > a").should("have.css", "min-width", "46px");
// Record alignment of collapsed GELS and messages on messagePanel
cy.get(".mx_MainSplit").percySnapshotElement("Collapsed GELS and messages on IRC layout", { percyCSS });

// 2. Alignment of expanded GELS and messages
// Click "expand" link button
cy.get(".mx_GenericEventListSummary_toggle[aria-expanded=false]").click();
// Check inline start spacing of info line on expanded GELS
cy.get(".mx_EventTile[data-layout=irc].mx_EventTile_info:first-of-type .mx_EventTile_line")
// See: _EventTile.pcss
// --EventTile_irc_line_info-margin-inline-start
// = 80 + 14 + 1 * 5
.should("have.css", "margin-inline-start", "99px");
// Record alignment of expanded GELS and messages on messagePanel
cy.get(".mx_MainSplit").percySnapshotElement("Expanded GELS and messages on IRC layout", { percyCSS });

// 3. Alignment of expanded GELS and placeholder of deleted message
// Delete the second (last) message
cy.get(".mx_RoomView_MessageList > .mx_EventTile_last").realHover();
cy.get(".mx_RoomView_MessageList > .mx_EventTile_last .mx_MessageActionBar_optionsButton", {
timeout: 1000,
})
.should("exist")
.realHover()
.click({ force: false });
cy.get(".mx_IconizedContextMenu_item[aria-label=Remove]").should("be.visible").click({ force: false });
// Confirm deletion
cy.get(".mx_Dialog_buttons button[data-testid=dialog-primary-button]")
.should("have.text", "Remove")
.click({ force: false });
// Make sure the dialog was closed and the second (last) message was redacted
cy.get(".mx_Dialog").should("not.exist");
cy.get(".mx_GenericEventListSummary .mx_EventTile_last .mx_RedactedBody").should("be.visible");
cy.get(".mx_GenericEventListSummary .mx_EventTile_last .mx_EventTile_receiptSent").should("be.visible");
// Record alignment of expanded GELS and placeholder of deleted message on messagePanel
cy.get(".mx_MainSplit").percySnapshotElement("Expanded GELS and with placeholder of deleted message", {
percyCSS,
});

// 4. Alignment of expanded GELS, placeholder of deleted message, and emote
// Send a emote
cy.get(".mx_RoomView_body .mx_BasicMessageComposer_input").type("/me says hello to Mr. Bot{enter}");
// Check inline start margin of its avatar
// Here --right-padding is for the avatar on the message line
// See: _IRCLayout.pcss
// .mx_IRCLayout .mx_EventTile_emote .mx_EventTile_avatar
// = calc(var(--name-width) + var(--icon-width) + 1 * var(--right-padding))
// = 80 + 14 + 1 * 5
cy.get(".mx_EventTile_emote .mx_EventTile_avatar").should("have.css", "margin-left", "99px");
// Make sure emote was sent
cy.get(".mx_EventTile_last.mx_EventTile_emote .mx_EventTile_receiptSent").should("be.visible");
// Record alignment of expanded GELS, placeholder of deleted message, and emote
cy.get(".mx_MainSplit").percySnapshotElement(
"Expanded GELS and with emote and placeholder of deleted message",
{
percyCSS,
},
);
});

it("should set inline start padding to a hidden event line", () => {
sendEvent(roomId);
cy.visit("/#/room/" + roomId);
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