Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chat backup - first part #1913

Merged
merged 2 commits into from
Oct 19, 2023
Merged

Chat backup - first part #1913

merged 2 commits into from
Oct 19, 2023

Conversation

stefanceriu
Copy link
Member

@stefanceriu stefanceriu commented Oct 17, 2023

This PR implements the screens and mocked logic for:

  • enabling/disabling key backup
  • generating, changing and verifying a recovery key
  • showing badges on the user avatar, settings option, settings screen chat backup and recovery section

The logic inside the SecureBackupController is currently mocked and goes like this:

  • backup enabled by default -> disable -> enable -> repeat
  • recovery disabled by default -> enable -> change -> fix -> change -> fix etc.

Designs here https://www.figma.com/file/0MMNu7cTOzLOlWb7ctTkv3/Element-X?type=design&node-id=12124-116601&mode=design&t=RLlU4wtjLjUt2xrO-0

Touches:
#1899
#1900
#1901
#1901

Screenshot 2023-10-20 at 14 40 36 Screenshot 2023-10-20 at 14 40 38 Screenshot 2023-10-20 at 14 40 41

@stefanceriu stefanceriu requested a review from pixlwave October 17, 2023 14:33
@stefanceriu stefanceriu requested a review from a team as a code owner October 17, 2023 14:33
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

Warnings
⚠️ This pull request seems relatively large. Please consider splitting it into multiple smaller ones.
⚠️ Please add a changelog.
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 Danger Swift against 6e26202

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Woah, great work!

@stefanceriu stefanceriu requested a review from pixlwave October 19, 2023 07:13
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

LGTM, (plus the changes we went over this morning).

@stefanceriu stefanceriu force-pushed the stefan/keyBackup branch 2 times, most recently from 42d28e1 to fe3b843 Compare October 19, 2023 11:53
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@stefanceriu stefanceriu enabled auto-merge (squash) October 19, 2023 12:33
@stefanceriu stefanceriu disabled auto-merge October 19, 2023 12:33
@stefanceriu stefanceriu merged commit c8b9d95 into develop Oct 19, 2023
4 of 5 checks passed
@stefanceriu stefanceriu deleted the stefan/keyBackup branch October 19, 2023 12:34
@callumu
Copy link

callumu commented Oct 24, 2023

Some UI comments:

  • Red dot on avatar - could this be 8x8 and with a 2px border between it and the avatar photo?
  • What colour is the key icon on the Chat backup menu? Could it be made consistent with the rest?
  • Learn more link - can this take the user to the section of the page where the title is 'What is Key Backup?'
  • Large key icon at the top of the screen - can this colour be changed to the color/icon/primary
  • Set up recovery screen - updated supporting copy
  • Recovery key UI - can the background be added behind 'Generate your recovery key' and the recovery key? Can the height of the box stay the same before and after the key has been generated? This is the same feedback for the 'Change recovery key?' page
  • Tap to copy recovery key - can the entire box be made tappable, not just the icon?
  • Save recovery key button - can this be the secondary colour? It should also be spaced 16px from the primary 'Done' button

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants