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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate to ActivityResultContracts #910

Merged
merged 22 commits into from Jul 3, 2020
Merged

Migrate to ActivityResultContracts #910

merged 22 commits into from Jul 3, 2020

Conversation

msfjarvis
Copy link
Member

@msfjarvis msfjarvis commented Jul 1, 2020

馃摙 Type of change

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

馃摐 Description

Migrates some nearly all uses of setTargetFragment and startActivityForResult to ActivityResultContracts.

馃挕 Motivation and Context

ActivityResultContracts is a cleaner and more effective abstraction and supports a lot more things than just startActivityForResult OOTB.

馃挌 How did you test it?

Manually verify app works as before

馃摑 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

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
I don't want to ever touch these files

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis added this to the 1.10.0 milestone Jul 1, 2020
@msfjarvis msfjarvis marked this pull request as draft July 1, 2020 16:29
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Member Author

@FabianHenneke this did resolve our problem with sync not calling refreshPasswordList and I also fixed a bug that I missed in #909

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis marked this pull request as ready for review July 1, 2020 18:11
@msfjarvis
Copy link
Member Author

I'm not going to touch the legacy Autofill implementation, and I can't think of a good way to eliminate the onActivityResult in BaseGitActivity.

@msfjarvis msfjarvis changed the title Start migrating to ActivityResultContracts Migrate to ActivityResultContracts Jul 1, 2020
- Silently delete empty directory
- Always refresh password list upon completion

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@fmeum
Copy link
Member

fmeum commented Jul 2, 2020

I'm not going to touch the legacy Autofill implementation, and I can't think of a good way to eliminate the onActivityResult in BaseGitActivity.

Me neither, we can tackle it some time after the GitAsyncTask migration.

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
* develop:
  build: upgrade Gradle wrapper (#911)
* develop:
  Fix up URIish instances with @ in user name (#913)
* develop:
  Fill OTP fields with SMS codes (#900)
@msfjarvis
Copy link
Member Author

@FabianHenneke this is good to go, I don't see anything I'd like to change.

Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

I will take a deeper look tomorrow.

* develop:
  release: script improvements (#915)
Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

Went through all the files and found the changes to be consistent. Well done!

@fmeum fmeum merged commit 25b4261 into android-password-store:develop Jul 3, 2020
@msfjarvis msfjarvis deleted the refactor/deprecation branch July 3, 2020 07:25
msfjarvis added a commit to fmeum/Android-Password-Store that referenced this pull request Jul 14, 2020
* 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>
@fmeum fmeum mentioned this pull request Jul 19, 2020
8 tasks
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.

None yet

3 participants