From afb2cb93f22d87d84edda5a1961f18e7f761dd45 Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Mon, 27 Mar 2023 21:13:25 +0900 Subject: [PATCH] Fix flaky Percy tests of `ReplyChain` (#10450) * Fix flaky Percy tests of ReplyChain - Add media query for percy on _ReplyChain.pcss to apply the same color to vertical strokes (border-left)of ReplyChain - Use CSS variables for visibility - Manage those variables on _common.pcss for maintainability Signed-off-by: Suguru Hirahara * Check receptSent as well for consistency Signed-off-by: Suguru Hirahara * Add a comment Signed-off-by: Suguru Hirahara * Specify zero spacing and remove list-style Signed-off-by: Suguru Hirahara --------- Signed-off-by: Suguru Hirahara --- cypress/e2e/timeline/timeline.spec.ts | 27 +++++++++---------- res/css/_common.pcss | 8 ++++++ res/css/views/avatars/_BaseAvatar.pcss | 2 +- res/css/views/elements/_ReplyChain.pcss | 8 ++++++ .../views/messages/_DisambiguatedProfile.pcss | 2 +- res/css/views/rooms/_ReplyTile.pcss | 13 +++++++++ 6 files changed, 44 insertions(+), 16 deletions(-) diff --git a/cypress/e2e/timeline/timeline.spec.ts b/cypress/e2e/timeline/timeline.spec.ts index 947eb7a38e..c9f05e0e2b 100644 --- a/cypress/e2e/timeline/timeline.spec.ts +++ b/cypress/e2e/timeline/timeline.spec.ts @@ -848,41 +848,41 @@ describe("Timeline", () => { clickButtonReply(); cy.getComposer().type(`${reply2}{enter}`); - // Make sure 'reply2' was sent + // Assert that 'reply2' was sent cy.contains(".mx_RoomView_MessageList .mx_EventTile_last", reply2).should("exist"); + cy.get(".mx_EventTile_last .mx_EventTile_receiptSent").should("be.visible"); // Exclude timestamp and read marker from snapshot - //const percyCSS = ".mx_MessageTimestamp, .mx_RoomView_myReadMarker { visibility: hidden !important; }"; + const percyCSS = ".mx_MessageTimestamp, .mx_RoomView_myReadMarker { visibility: hidden !important; }"; // Check the margin value of ReplyChains of EventTile at the bottom on IRC layout cy.setSettingValue("layout", null, SettingLevel.DEVICE, Layout.IRC); cy.get(".mx_EventTile_last[data-layout='irc'] .mx_ReplyChain").should("have.css", "margin", "0px"); // Take a snapshot on IRC layout - // Disabled because flaky - see https://github.com/vector-im/element-web/issues/24881 - /*cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on IRC layout", { + // Note that because zero margin is applied to mx_ReplyChain, the left borders of two mx_ReplyChain + // components may seem to be connected to one. + cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on IRC layout", { percyCSS, - });*/ + }); // Check the margin value of ReplyChains of EventTile at the bottom on group/modern layout cy.setSettingValue("layout", null, SettingLevel.DEVICE, Layout.Group); cy.get(".mx_EventTile_last[data-layout='group'] .mx_ReplyChain").should("have.css", "margin-bottom", "8px"); // Take a snapshot on modern layout - // Disabled because flaky - see https://github.com/vector-im/element-web/issues/24881 - /*cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on modern layout", { + cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on modern layout", { percyCSS, - });*/ + }); // Check the margin value of ReplyChains of EventTile at the bottom on group/modern compact layout cy.setSettingValue("useCompactLayout", null, SettingLevel.DEVICE, true); cy.get(".mx_EventTile_last[data-layout='group'] .mx_ReplyChain").should("have.css", "margin-bottom", "4px"); // Take a snapshot on compact modern layout - // Disabled because flaky - see https://github.com/vector-im/element-web/issues/24881 - /*cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on compact modern layout", { + cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on compact modern layout", { percyCSS, - });*/ + }); // Check the margin value of ReplyChains of EventTile at the bottom on bubble layout cy.setSettingValue("layout", null, SettingLevel.DEVICE, Layout.Bubble); @@ -893,10 +893,9 @@ describe("Timeline", () => { ); // Take a snapshot on bubble layout - // Disabled because flaky - see https://github.com/vector-im/element-web/issues/24881 - /*cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on bubble layout", { + cy.get(".mx_EventTile_last").percySnapshotElement("EventTile with reply chains on bubble layout", { percyCSS, - });*/ + }); }); it("should send, reply, and display long strings without overflowing", () => { diff --git a/res/css/_common.pcss b/res/css/_common.pcss index bec6fe8844..754b4dc253 100644 --- a/res/css/_common.pcss +++ b/res/css/_common.pcss @@ -45,6 +45,14 @@ $timeline-image-border-radius: 8px; --MessageTimestamp-width: $MessageTimestamp_width; } +@media only percy { + :root { + --percy-color-avatar: $username-variant2-color; + --percy-color-displayName: $username-variant1-color; + --percy-color-replyChain-border: $username-variant1-color; + } +} + @media (prefers-reduced-motion) { :root { --transition-short: 0; diff --git a/res/css/views/avatars/_BaseAvatar.pcss b/res/css/views/avatars/_BaseAvatar.pcss index a6a4b0b74b..70c41d0b25 100644 --- a/res/css/views/avatars/_BaseAvatar.pcss +++ b/res/css/views/avatars/_BaseAvatar.pcss @@ -58,7 +58,7 @@ limitations under the License. @media only percy { /* Stick the default room avatar colour, so it doesn't cause a false diff on the screenshot */ .mx_BaseAvatar_initial { - background-color: $username-variant2-color !important; + background-color: var(--percy-color-avatar) !important; border-radius: 125px; } .mx_RoomAvatar_isSpaceRoom .mx_BaseAvatar_initial { diff --git a/res/css/views/elements/_ReplyChain.pcss b/res/css/views/elements/_ReplyChain.pcss index 090a0bcbe8..6de9d367a2 100644 --- a/res/css/views/elements/_ReplyChain.pcss +++ b/res/css/views/elements/_ReplyChain.pcss @@ -64,3 +64,11 @@ limitations under the License. --username-color: $username-variant8-color; } } + +/* Percy screenshot test specific CSS */ +@media only percy { + .mx_ReplyChain { + /* Override the colour in percy tests for screenshot consistency */ + border-left-color: var(--percy-color-replyChain-border) !important; + } +} diff --git a/res/css/views/messages/_DisambiguatedProfile.pcss b/res/css/views/messages/_DisambiguatedProfile.pcss index 4863dc3518..ab173f27b1 100644 --- a/res/css/views/messages/_DisambiguatedProfile.pcss +++ b/res/css/views/messages/_DisambiguatedProfile.pcss @@ -38,6 +38,6 @@ limitations under the License. @media only percy { .mx_DisambiguatedProfile_displayName { /* Override the colour in percy tests for screenshot consistency */ - color: $username-variant1-color !important; + color: var(--percy-color-displayName) !important; } } diff --git a/res/css/views/rooms/_ReplyTile.pcss b/res/css/views/rooms/_ReplyTile.pcss index 1a2e1a3814..d25a086965 100644 --- a/res/css/views/rooms/_ReplyTile.pcss +++ b/res/css/views/rooms/_ReplyTile.pcss @@ -139,3 +139,16 @@ limitations under the License. } } } + +@media only Percy { + /* Remove the list style in percy tests for screenshot consistency */ + :is(ul, ol) { + padding: 0 !important; + margin: 0 !important; + list-style: none !important; + + .mx_EventTile_last { + padding: 0 !important; + } + } +}