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

Fix style inconsistencies of "Verify this device" dialog #8167

Closed
wants to merge 16 commits into from
Closed

Fix style inconsistencies of "Verify this device" dialog #8167

wants to merge 16 commits into from

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Mar 27, 2022

Closes element-hq/element-web#21572

This PR addresses style inconsistencies of the security key dialog and modal window, including

  • Window's padding value (based on authentication modal window; see Image 1)
  • Border-radius (see Image 1)
  • Text color (based on Secure Key dialog; see Image 2)
  • The warning sign of the footer message (see Image 2)
  • Size of the close button (see image 2)

Signed-off-by: Suguru Hirahara [email protected]

Before

Current design
before2

Image 1
after1

Image 2
after

After

after2

Perhaps the big red icon on the header should be replaced with something else.

after

type: defect


Here's what your changelog entry will look like:

🐛 Bug Fixes

Preview: https://pr8167--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@luixxiul luixxiul requested a review from a team as a code owner March 27, 2022 13:17
@github-actions github-actions bot added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Mar 27, 2022
@codecov
Copy link

codecov bot commented Mar 27, 2022

Codecov Report

Merging #8167 (385b441) into develop (61076c3) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##           develop    #8167   +/-   ##
========================================
  Coverage    29.25%   29.25%           
========================================
  Files          863      863           
  Lines        49876    49876           
  Branches     12696    12696           
========================================
  Hits         14592    14592           
  Misses       35284    35284           
Impacted Files Coverage Δ
...rc/components/structures/auth/CompleteSecurity.tsx 0.00% <ø> (ø)
...components/structures/auth/SetupEncryptionBody.tsx 0.00% <0.00%> (ø)

@SimonBrandner
Copy link
Contributor

@luixxiul, could you please use Fixes <issue link>, so that the PR shows up in the issue sidebar as a PR that might fix the issue?

@luixxiul luixxiul marked this pull request as draft March 29, 2022 15:58
@luixxiul
Copy link
Contributor Author

No problem. Considering how much change this PR should include, I've replaced "fixes" with "addresses".

@luixxiul luixxiul changed the title Fix style inconsistency of "Reset all" footers Fix style inconsistency of "Verify this device" dialog Mar 29, 2022
Signed-off-by: Suguru Hirahara <[email protected]>
- Nesting

Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
Signed-off-by: Suguru Hirahara <[email protected]>
@luixxiul luixxiul changed the title Fix style inconsistency of "Verify this device" dialog Fix style inconsistencies of "Verify this device" dialog Mar 29, 2022
@luixxiul luixxiul marked this pull request as ready for review March 29, 2022 20:03
@@ -18,4 +18,5 @@ limitations under the License.
@mixin ButtonResetWarning;
color: $light-fg-color; // .mx_Dialog on _common.scss
margin-top: $font-14px;
line-height: 1; // align the warning icon
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change should fix this bug:

after (copy 1)

@luixxiul
Copy link
Contributor Author

luixxiul commented Apr 1, 2022

There is another dialog whose styles are also inconsistent due to the fact it uses BaseDialog.

before

Here the color of the warning icon is black.

@luixxiul luixxiul marked this pull request as draft April 1, 2022 21:12
@luixxiul
Copy link
Contributor Author

luixxiul commented Apr 2, 2022

Since it looks like it needs complicated changes of SetupEncryptionDialog.tsx more than changing some values of scss, I would like to leave it as it is for now.

@luixxiul luixxiul marked this pull request as ready for review April 2, 2022 05:30
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.

I think this one needs design review before code review is possible.

@turt2live turt2live requested a review from a team April 4, 2022 06:19
@MadLittleMods MadLittleMods added the Z-Community-PR Issue is solved by a community member's PR label Jun 1, 2022
@amshakal amshakal requested review from amshakal and removed request for a team July 13, 2022 15:51
@luixxiul
Copy link
Contributor Author

Closing as stale.

@luixxiul luixxiul closed this Mar 14, 2023
@luixxiul luixxiul deleted the ButtonResetWarning branch March 14, 2023 08:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

Successfully merging this pull request may close these issues.

Design of secret key dialogs inconsistent
4 participants