From 232daaff68398ec4e1d6973b7e91b9b033b9a10c Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Thu, 30 Mar 2023 18:11:16 +0900 Subject: [PATCH] Fix decryption failure bar covering the timeline (#10360) * Use grid layout instead - BEM naming style - Increase block gap from 4px to 8px - Use flexbox inside 'header' grid-area to let the buttons wrapped - Use variables - Remove 4px gap when one of the buttons is not rendered - Change 'body' to 'message' - Set 'align-self: start' to the icon and spinner Signed-off-by: Suguru Hirahara * Unset height of spinner Signed-off-by: Suguru Hirahara * Break lines at newline characters with white-space: pre-line Signed-off-by: Suguru Hirahara * Edit tests to check decryption failure bars on narrow timeline - checkTimelineNarrow() looks for buttons by default - Test indicator as well Signed-off-by: Suguru Hirahara * Remove a line Signed-off-by: Suguru Hirahara * Edit the test to have it check mx_EventTile_last only inside mx_RoomView_body Signed-off-by: Suguru Hirahara * Fix double underscores Signed-off-by: Suguru Hirahara * Fix double underscores - pcss Signed-off-by: Suguru Hirahara * Iterate - buttons at the bottom - Set common spacing to buttons with variables - Remove line breaks, yarn run i18n - Set data-testid for headlines and buttons in case the tested strings would be displayed elsewhere simultaneously Signed-off-by: Suguru Hirahara * Check waiting headline as well Signed-off-by: Suguru Hirahara * Increase spacing between the message and the buttons Signed-off-by: Suguru Hirahara * lint Signed-off-by: Suguru Hirahara * Increase block gap between wrapped buttons for clickability Apply 8px between wrapped buttons Signed-off-by: Suguru Hirahara * Revert bottom margin of buttons which are not expected to be wrapped Signed-off-by: Suguru Hirahara * Check visibility instead of existence This commit removes data-testid from headlines and data-testid-button and checks whether the elements are really visible, not overflowing the viewport. Signed-off-by: Suguru Hirahara * Remove redundant gap between 'mx_DecryptionFailureBar_start' and the bottom edge This commit adds '.mx_DecryptionFailureBar--withEnd' class name to have it applied to the bar only if it has button(s). This way the bar is rendered with a flexbox and the row-gap declaration is respected only if there is a 'mx_DecryptionFailureBar--withEnd' element. The element currently includes the button(s) only. Signed-off-by: Suguru Hirahara * lint - prettier Signed-off-by: Suguru Hirahara * Have Percy take a snapshot of the bar loading spinner before checkTimelineNarrow() The loading spinner is likely to disappear while checking the bar on the narrow timeline. Signed-off-by: Suguru Hirahara --------- Signed-off-by: Suguru Hirahara Co-authored-by: Kerry --- cypress/e2e/crypto/decryption-failure.spec.ts | 73 ++- res/css/_common.pcss | 8 +- .../views/rooms/_DecryptionFailureBar.pcss | 93 ++-- .../views/rooms/DecryptionFailureBar.tsx | 77 ++- .../DecryptionFailureBar-test.tsx.snap | 458 +++++++++++------- 5 files changed, 471 insertions(+), 238 deletions(-) diff --git a/cypress/e2e/crypto/decryption-failure.spec.ts b/cypress/e2e/crypto/decryption-failure.spec.ts index 15b437d621..1583752686 100644 --- a/cypress/e2e/crypto/decryption-failure.spec.ts +++ b/cypress/e2e/crypto/decryption-failure.spec.ts @@ -57,6 +57,28 @@ const handleVerificationRequest = (request: VerificationRequest): Chainable { + cy.viewport(800, 600); // SVGA + cy.get(".mx_LeftPanel_minimized").should("exist"); // Wait until the left panel is minimized + cy.get(".mx_RightPanel_roomSummaryButton").click(); // Open the right panel to make the timeline narrow + cy.get(".mx_BaseCard").should("exist"); + + // Ensure the failure bar does not cover the timeline + cy.get(".mx_RoomView_body .mx_EventTile.mx_EventTile_last").should("be.visible"); + + // Ensure the indicator does not overflow the timeline + cy.get("[data-testid='decryption-failure-bar-indicator']").should("be.visible"); + + if (button) { + // Ensure the button does not overflow the timeline + cy.get("[data-testid='decryption-failure-bar-button']:last-of-type").should("be.visible"); + } + + cy.get(".mx_RightPanel_roomSummaryButton").click(); // Close the right panel + cy.get(".mx_BaseCard").should("not.exist"); + cy.viewport(1000, 660); // Reset to the default size +}; + describe("Decryption Failure Bar", () => { let homeserver: HomeserverInstance | undefined; let testUser: UserCredentials | undefined; @@ -113,10 +135,13 @@ describe("Decryption Failure Bar", () => { }) .then(() => { cy.botSendMessage(bot, roomId, "test"); + cy.contains(".mx_DecryptionFailureBar_start_headline", "Decrypting messages…").should("be.visible"); cy.contains( - ".mx_DecryptionFailureBar .mx_DecryptionFailureBar_message_headline", + ".mx_DecryptionFailureBar_start_headline", "Verify this device to access all messages", - ); + ).should("be.visible"); + + checkTimelineNarrow(); cy.get(".mx_DecryptionFailureBar").percySnapshotElement( "DecryptionFailureBar prompts user to verify", @@ -125,12 +150,14 @@ describe("Decryption Failure Bar", () => { }, ); - cy.contains(".mx_DecryptionFailureBar_button", "Resend key requests").should("not.exist"); - cy.contains(".mx_DecryptionFailureBar_button", "Verify").click(); + cy.contains(".mx_DecryptionFailureBar_end", "Resend key requests").should("not.exist"); + cy.contains(".mx_DecryptionFailureBar_end", "Verify").should("be.visible").click(); const verificationRequestPromise = waitForVerificationRequest(otherDevice); cy.get(".mx_CompleteSecurity_actionRow .mx_AccessibleButton").click(); - cy.contains("To proceed, please accept the verification request on your other device."); + cy.contains("To proceed, please accept the verification request on your other device.").should( + "be.visible", + ); cy.wrap(verificationRequestPromise).then((verificationRequest: VerificationRequest) => { cy.wrap(verificationRequest.accept()); handleVerificationRequest(verificationRequest).then((emojis) => { @@ -146,10 +173,12 @@ describe("Decryption Failure Bar", () => { cy.get(".mx_VerificationPanel_verified_section .mx_E2EIcon_verified").should("exist"); cy.contains(".mx_AccessibleButton", "Got it").click(); - cy.get(".mx_DecryptionFailureBar .mx_DecryptionFailureBar_message_headline").should( - "have.text", + cy.contains( + ".mx_DecryptionFailureBar_start_headline", "Open another device to load encrypted messages", - ); + ).should("be.visible"); + + checkTimelineNarrow(); cy.get(".mx_DecryptionFailureBar").percySnapshotElement( "DecryptionFailureBar prompts user to open another device, with Resend Key Requests button", @@ -159,9 +188,12 @@ describe("Decryption Failure Bar", () => { ); cy.intercept("/_matrix/client/r0/sendToDevice/m.room_key_request/*").as("keyRequest"); - cy.contains(".mx_DecryptionFailureBar_button", "Resend key requests").click(); + cy.contains(".mx_DecryptionFailureBar_end_button", "Resend key requests").should("be.visible").click(); cy.wait("@keyRequest"); - cy.contains(".mx_DecryptionFailureBar_button", "Resend key requests").should("not.exist"); + cy.contains(".mx_DecryptionFailureBar_end_button", "Resend key requests").should("not.exist"); + cy.contains(".mx_DecryptionFailureBar_end_button", "View your device list").should("be.visible"); + + checkTimelineNarrow(); cy.get(".mx_DecryptionFailureBar").percySnapshotElement( "DecryptionFailureBar prompts user to open another device, without Resend Key Requests button", @@ -184,15 +216,17 @@ describe("Decryption Failure Bar", () => { cy.botSendMessage(bot, roomId, "test"); cy.contains( - ".mx_DecryptionFailureBar .mx_DecryptionFailureBar_message_headline", + ".mx_DecryptionFailureBar_start_headline", "Reset your keys to prevent future decryption errors", - ); + ).should("be.visible"); + + checkTimelineNarrow(); cy.get(".mx_DecryptionFailureBar").percySnapshotElement("DecryptionFailureBar prompts user to reset keys", { widths: [320, 640], }); - cy.contains(".mx_DecryptionFailureBar_button", "Reset").click(); + cy.contains(".mx_DecryptionFailureBar_end_button", "Reset").should("be.visible").click(); // Set up key backup cy.get(".mx_Dialog").within(() => { @@ -204,11 +238,12 @@ describe("Decryption Failure Bar", () => { cy.contains("Done").click(); }); - cy.get(".mx_DecryptionFailureBar .mx_DecryptionFailureBar_message_headline").should( - "have.text", - "Some messages could not be decrypted", + cy.contains(".mx_DecryptionFailureBar_start_headline", "Some messages could not be decrypted").should( + "be.visible", ); + checkTimelineNarrow(false); // button should not be rendered here + cy.get(".mx_DecryptionFailureBar").percySnapshotElement( "DecryptionFailureBar displays general message with no call to action", { @@ -233,9 +268,11 @@ describe("Decryption Failure Bar", () => { widths: [320, 640], }); + checkTimelineNarrow(); + cy.wait(5000); cy.get(".mx_DecryptionFailureBar .mx_Spinner").should("not.exist"); - cy.get(".mx_DecryptionFailureBar .mx_DecryptionFailureBar_icon").should("exist"); + cy.get("[data-testid='decryption-failure-bar-icon']").should("be.visible"); cy.get(".mx_RoomView_messagePanel").scrollTo("top"); cy.get(".mx_DecryptionFailureBar").should("not.exist"); @@ -245,5 +282,7 @@ describe("Decryption Failure Bar", () => { cy.get(".mx_RoomView_messagePanel").scrollTo("bottom"); cy.get(".mx_DecryptionFailureBar").should("exist"); + + checkTimelineNarrow(); }); }); diff --git a/res/css/_common.pcss b/res/css/_common.pcss index 754b4dc253..edc3042332 100644 --- a/res/css/_common.pcss +++ b/res/css/_common.pcss @@ -43,6 +43,8 @@ $timeline-image-border-radius: 8px; --transition-short: 0.1s; --transition-standard: 0.3s; --MessageTimestamp-width: $MessageTimestamp_width; + --buttons-dialog-gap-row: $spacing-8; + --buttons-dialog-gap-column: $spacing-8; } @media only percy { @@ -525,8 +527,8 @@ legend { margin-inline-start: auto; /* default gap among elements */ - column-gap: $spacing-8; /* See margin-right below inside the button style */ - row-gap: 5px; /* See margin-bottom below inside the button style */ + column-gap: var(--buttons-dialog-gap-column); + row-gap: var(--buttons-dialog-gap-row); button { margin: 0 !important; /* override the margin settings */ @@ -548,7 +550,7 @@ legend { .mx_Dialog_buttons input[type="submit"] { @mixin mx_DialogButton; margin-left: 0px; - margin-right: 8px; + margin-right: var(--buttons-dialog-gap-column); margin-bottom: 5px; /* flip colours for the secondary ones */ diff --git a/res/css/views/rooms/_DecryptionFailureBar.pcss b/res/css/views/rooms/_DecryptionFailureBar.pcss index 1369d599e3..57dc71b731 100644 --- a/res/css/views/rooms/_DecryptionFailureBar.pcss +++ b/res/css/views/rooms/_DecryptionFailureBar.pcss @@ -15,46 +15,73 @@ limitations under the License. */ .mx_DecryptionFailureBar { + --gap-row: $spacing-8; + --gap-column: $spacing-12; + --gap: var(--gap-row) var(--gap-column); + --size-icon: 24px; + background-color: $system; padding: $spacing-12; - margin-left: $spacing-16; - margin-right: $spacing-16; + margin-inline: $spacing-16; border-radius: 4px; - display: flex; - align-items: flex-start; - gap: $spacing-12; -} -.mx_DecryptionFailureBar_icon { - width: 24px; - height: 24px; - mask-image: url("$(res)/img/e2e/decryption-failure.svg"); - background-color: $e2e-warning-color; - mask-repeat: no-repeat; - mask-position: center; - mask-size: contain; -} + &.mx_DecryptionFailureBar--withEnd { + display: flex; + flex-flow: wrap; + align-items: flex-start; + justify-content: space-between; + row-gap: calc(var(--gap-row) + $spacing-4); /* Increase spacing between the message and the buttons */ -.mx_DecryptionFailureBar_icon, -.mx_DecryptionFailureBar .mx_Spinner { - flex-shrink: 0; - flex-grow: 0; -} + .mx_DecryptionFailureBar_end { + display: flex; + flex-wrap: wrap; /* Let the buttons wrapped on a narrow column */ + gap: var(--buttons-dialog-gap-row) var(--buttons-dialog-gap-column); + margin-inline-start: calc(var(--size-icon) + var(--gap-column)); /* Align the button(s) and the message */ + } + } -.mx_DecryptionFailureBar_message { - flex-grow: 1; -} + .mx_DecryptionFailureBar_start { + display: grid; + gap: var(--gap); + grid-template-areas: + "status headline" + ". message"; + grid-template-columns: var(--size-icon) auto; -.mx_DecryptionFailureBar_message_headline { - font-weight: $font-semi-bold; - font-size: $font-16px; - margin-bottom: $spacing-4; -} + .mx_DecryptionFailureBar_start_status { + grid-area: status; -.mx_DecryptionFailureBar_message_body { - color: $secondary-content; -} + display: flex; + align-items: center; + gap: var(--gap); -.mx_DecryptionFailureBar_button { - flex-shrink: 0; + .mx_Spinner { + height: unset; /* Unset height: 100% */ + } + + .mx_DecryptionFailureBar_start_status_icon { + min-width: var(--size-icon); + height: var(--size-icon); + mask-image: url("$(res)/img/e2e/decryption-failure.svg"); + background-color: $e2e-warning-color; + mask-repeat: no-repeat; + mask-position: center; + mask-size: contain; + } + } + + .mx_DecryptionFailureBar_start_headline { + grid-area: headline; + + font-weight: $font-semi-bold; + font-size: $font-16px; + align-self: center; + } + + .mx_DecryptionFailureBar_start_message { + grid-area: message; + + color: $secondary-content; + } + } } diff --git a/src/components/views/rooms/DecryptionFailureBar.tsx b/src/components/views/rooms/DecryptionFailureBar.tsx index 7849f63853..94cf02d8a6 100644 --- a/src/components/views/rooms/DecryptionFailureBar.tsx +++ b/src/components/views/rooms/DecryptionFailureBar.tsx @@ -145,34 +145,47 @@ export const DecryptionFailureBar: React.FC = ({ failures }) => { store.resetConfirm(); }; - const statusIndicator = waiting ? :
; + const statusIndicator = waiting ? ( + + ) : ( +
+ ); + let className; let headline: JSX.Element; - let body: JSX.Element; + let message: JSX.Element; let button = ; if (waiting) { + className = "mx_DecryptionFailureBar"; headline = {_t("Decrypting messages…")}; - body = ( + message = ( {_t("Please wait as we try to decrypt your messages. This may take a few moments.")} ); } else if (needsVerification) { if (hasOtherVerifiedDevices || hasKeyBackup) { + className = "mx_DecryptionFailureBar mx_DecryptionFailureBar--withEnd"; headline = {_t("Verify this device to access all messages")}; - body = ( + message = ( {_t("This device was unable to decrypt some messages because it has not been verified yet.")} ); button = ( - + {_t("Verify")} ); } else { + className = "mx_DecryptionFailureBar mx_DecryptionFailureBar--withEnd"; headline = {_t("Reset your keys to prevent future decryption errors")}; - body = ( + message = ( {_t( "You will not be able to access old undecryptable messages, " + @@ -181,14 +194,20 @@ export const DecryptionFailureBar: React.FC = ({ failures }) => { ); button = ( - + {_t("Reset")} ); } } else if (hasOtherVerifiedDevices) { + className = "mx_DecryptionFailureBar mx_DecryptionFailureBar--withEnd"; headline = {_t("Open another device to load encrypted messages")}; - body = ( + message = ( {_t( "This device is requesting decryption keys from your other devices. " + @@ -197,13 +216,19 @@ export const DecryptionFailureBar: React.FC = ({ failures }) => { ); button = ( - + {_t("View your device list")} ); } else { + className = "mx_DecryptionFailureBar"; headline = {_t("Some messages could not be decrypted")}; - body = ( + message = ( {_t( "Unfortunately, there are no other verified devices to request decryption keys from. " + @@ -215,24 +240,32 @@ export const DecryptionFailureBar: React.FC = ({ failures }) => { let keyRequestButton = ; if (!needsVerification && hasOtherVerifiedDevices && anyUnrequestedSessions) { + className = "mx_DecryptionFailureBar mx_DecryptionFailureBar--withEnd"; keyRequestButton = ( -
- - {_t("Resend key requests")} - -
+ + {_t("Resend key requests")} + ); } return ( -
- {statusIndicator} -
-
{headline}
-
{body}
+
+
+
+
{statusIndicator}
+
+
{headline}
+
{message}
+
+
+ {button} + {keyRequestButton}
-
{button}
- {keyRequestButton}
); }; diff --git a/test/components/views/rooms/__snapshots__/DecryptionFailureBar-test.tsx.snap b/test/components/views/rooms/__snapshots__/DecryptionFailureBar-test.tsx.snap index 76adfef8b5..35b95a37f4 100644 --- a/test/components/views/rooms/__snapshots__/DecryptionFailureBar-test.tsx.snap +++ b/test/components/views/rooms/__snapshots__/DecryptionFailureBar-test.tsx.snap @@ -5,24 +5,33 @@ exports[` Displays a general error message if there are class="mx_DecryptionFailureBar" >
-
+
+
+
+
+
Some messages could not be decrypted
Unfortunately, there are no other verified devices to request decryption keys from. Signing in and verifying other devices may help avoid this situation in the future.
`; @@ -32,73 +41,88 @@ exports[` Displays a loading spinner 1`] = ` class="mx_DecryptionFailureBar" >
-
-
+ class="mx_DecryptionFailureBar_start_status" + > +
+
+
+
+
+
Decrypting messages…
Please wait as we try to decrypt your messages. This may take a few moments.
`; exports[` Displays button to resend key requests if we are verified 1`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
View your device list
-
-
@@ -110,30 +134,40 @@ exports[` Displays button to resend key requests if we a exports[` Displays button to resend key requests if we are verified 2`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
@@ -145,41 +179,48 @@ exports[` Displays button to resend key requests if we a exports[` Displays the button to resend key requests only if there are sessions we haven't already requested 1`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
View your device list
-
-
@@ -191,30 +232,40 @@ exports[` Displays the button to resend key requests onl exports[` Displays the button to resend key requests only if there are sessions we haven't already requested 2`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
@@ -226,41 +277,48 @@ exports[` Displays the button to resend key requests onl exports[` Displays the button to resend key requests only if there are sessions we haven't already requested 3`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
View your device list
-
-
@@ -272,30 +330,40 @@ exports[` Displays the button to resend key requests onl exports[` Displays the button to resend key requests only if there are sessions we haven't already requested 4`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
@@ -307,30 +375,40 @@ exports[` Displays the button to resend key requests onl exports[` Does not display a button to send key requests if we are unverified 1`] = `
-
+
+
+
+
+
Verify this device to access all messages
This device was unable to decrypt some messages because it has not been verified yet.
@@ -342,30 +420,40 @@ exports[` Does not display a button to send key requests exports[` Handles device updates 1`] = `
-
+
+
+
+
+
Verify this device to access all messages
This device was unable to decrypt some messages because it has not been verified yet.
@@ -377,41 +465,48 @@ exports[` Handles device updates 1`] = ` exports[` Handles device updates 2`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
View your device list
-
-
@@ -423,30 +518,40 @@ exports[` Handles device updates 2`] = ` exports[` Prompts the user to reset if they have no other verified devices and no backups 1`] = `
-
+
+
+
+
+
Reset your keys to prevent future decryption errors
You will not be able to access old undecryptable messages, but resetting your keys will allow you to receive new messages.
@@ -458,30 +563,40 @@ exports[` Prompts the user to reset if they have no othe exports[` Prompts the user to verify if they have backups 1`] = `
-
+
+
+
+
+
Verify this device to access all messages
This device was unable to decrypt some messages because it has not been verified yet.
@@ -493,30 +608,40 @@ exports[` Prompts the user to verify if they have backup exports[` Prompts the user to verify if they have other devices 1`] = `
-
+
+
+
+
+
Verify this device to access all messages
This device was unable to decrypt some messages because it has not been verified yet.
@@ -528,41 +653,48 @@ exports[` Prompts the user to verify if they have other exports[` Recommends opening other devices if there are other verified devices 1`] = `
-
+
+
+
+
+
Open another device to load encrypted messages
This device is requesting decryption keys from your other devices. Opening one of your other devices may speed this up.
View your device list
-
-