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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Move password export to the IO dispatcher #918

Merged
merged 13 commits into from Jul 9, 2020
Merged

Move password export to the IO dispatcher #918

merged 13 commits into from Jul 9, 2020

Conversation

msfjarvis
Copy link
Member

馃摙 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

馃摐 Description

Makes password export asynchronous and adds some UI feedback to make it visible to the user that something is happening in the background.

馃挕 Motivation and Context

Exporting stores with a large number of directories/files (~100? maybe even lesser?) causes the app to stall and freeze until the copy task is complete.

馃挌 How did you test it?

Manually exported passwords multiple times and the app did not freeze.

馃摑 Checklist

  • I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

馃敭 Next steps

馃摳 Screenshots / GIFs

export

@msfjarvis
Move password export to the IO dispatcher
6851937 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis added this to the 1.10.0 milestone Jul 4, 2020
@Skrilltrax
Copy link
Member

Can you reduce ProgressBar size a bit. That doesn't look good imo.

@Skrilltrax
Copy link
Member

Also doing this is lifecycleScope is kinda bad. If someone closes UserPreference that would kill the export.

@msfjarvis
Simplify snackbars and disable exit operations during export
eec9a48 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Member Author

Removed the progressbar. The export will fail if the activity dies so I've added some workarounds to prevent the user from killing it.

msfjarvis and others added 8 commits July 8, 2020 13:14
@msfjarvis
Merge branch 'develop' into improve-password-export
16e07bb 
* develop:
  styles: re-add alertDialogTheme override (#923)
  Work around Chrome Autofill issue (#921)
  Major UI overhaul and the introduction of a new icon (#920)

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Merge branch 'develop' into improve-password-export
5f388b6 
* develop:
  global: set an import order rule and reformat with it (#924)

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@Skrilltrax
Move export password logic to service
a4c6417 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@msfjarvis
Merge branch 'develop' into improve-password-export
66e1b24 
* develop:
  Mention #482 being fixed in the changelog (#925)

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Reformat
8b23cbe 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Use explicit null check
5714128 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Remove unneeded hack
2eb2c71 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Fixup strings
39f3406 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
msfjarvis
msfjarvis commented Jul 8, 2020
View reviewed changes
Copy link
Member Author

@msfjarvis msfjarvis left a comment

Choose a reason for hiding this comment

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

The app seems to hang for me completely on API 30 after directory selection, then ANRs, and finally crashes back to the settings screen.

@Skrilltrax
Copy link
Member

I've tested on API 29 and it was working fine. I'll test on emulator.

@Skrilltrax
Don't use coroutines in a service
575662a 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@Skrilltrax
Copy link
Member

@msfjarvis can you test it again?

@msfjarvis
Copy link
Member Author

@msfjarvis can you test it again?

Works now, but the icon needs to be changed. Try this.

@Skrilltrax
Update notification icon
f250fd7 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@msfjarvis
Copy link
Member Author

LGTM, approve and submit once presubmit runs finish.

@msfjarvis
Rollback unwanted formatting
d3ac055 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@Skrilltrax Skrilltrax merged commit fc00de6 into android-password-store:develop Jul 9, 2020
@msfjarvis msfjarvis deleted the improve-password-export branch July 9, 2020 08:32
msfjarvis added a commit that referenced this pull request Jul 9, 2020
@msfjarvis
Merge branch 'develop' into refactoring/exclude_third_party_reformat
fa4f7ba 
* develop:
  Move password export to the IO dispatcher (#918)
msfjarvis added a commit to fmeum/Android-Password-Store that referenced this pull request Jul 14, 2020
@msfjarvis
Merge branch 'develop' into fhenneke_sshj_keystore
89a9401 
* develop: (77 commits)
  Add debug icon and update color palette (android-password-store#931)
  Revert "Work around Chrome Autofill issue (android-password-store#921)" (android-password-store#933)
  github: remove freeDebug variant from pull request matrix (android-password-store#932)
  Properly guard against invalid renaming (android-password-store#929)
  Fix navigation bar theming and reformat (android-password-store#930)
  Exclude third_party scope from reformats (android-password-store#927)
  Move password export to the IO dispatcher (android-password-store#918)
  Mention android-password-store#482 being fixed in the changelog (android-password-store#925)
  global: set an import order rule and reformat with it (android-password-store#924)
  styles: re-add alertDialogTheme override (android-password-store#923)
  Work around Chrome Autofill issue (android-password-store#921)
  Major UI overhaul and the introduction of a new icon (android-password-store#920)
  Update Public Suffix List data (android-password-store#917)
  Migrate to ActivityResultContracts (android-password-store#910)
  release: script improvements (android-password-store#915)
  Deploy both variants to snapshot directory (android-password-store#914)
  Fill OTP fields with SMS codes (android-password-store#900)
  Fix up URIish instances with @ in user name (android-password-store#913)
  build: upgrade Gradle wrapper (android-password-store#911)
  Scroll to files and enter folders when created (android-password-store#909)
  ...

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis mentioned this pull request Aug 7, 2020
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmeum fmeum fmeum approved these changes

@Skrilltrax Skrilltrax Awaiting requested review from Skrilltrax

Assignees

@Skrilltrax Skrilltrax

Labels
None yet
Projects
None yet
Milestone
1.10.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@msfjarvis @Skrilltrax @fmeum