Skip to content

refactor(compose): Use registerForActivityResult for picking content - #833

Open
Inhishonor wants to merge 1 commit into
quik-sms:masterfrom
Inhishonor:update-file-picker
Open

refactor(compose): Use registerForActivityResult for picking content#833
Inhishonor wants to merge 1 commit into
quik-sms:masterfrom
Inhishonor:update-file-picker

Conversation

@Inhishonor

Copy link
Copy Markdown
Collaborator

For picking contacts, photos, and files, use the modern API's. Additionally, Android 17 forces using GetMultipleContents rather than ACTION_PICK, so update that.

Closes #831

startActivityForResult(Intent.createChooser(intent, null), requestCode)
override fun requestGallery() {
pickMedia.launch(
PickVisualMediaRequest(ActivityResultContracts.PickVisualMedia.ImageAndVideo)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It would be nice to do https://developer.android.com/reference/androidx/activity/result/contract/ActivityResultContracts.PickVisualMedia.DefaultTab.AlbumsTab here, but that would require updating compile sdk and adding a new dependency. So might have to wait.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the Compose screen’s attachment flows to use registerForActivityResult-based contracts for contacts, gallery media, and file picking, including a new Android 17+ path intended to fix the “attach any file” regression reported in #831.

Changes:

  • Replace legacy startActivityForResult flows for picking contacts/files with Activity Result APIs.
  • Introduce separate view hooks for gallery picking vs. generic file picking (requestGallery() / requestFilePicker()).
  • Use GetMultipleContents for file picking on Android 17+ while preserving the older chooser flow on earlier versions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
presentation/src/main/java/com/moez/QKSMS/feature/compose/ComposeViewModel.kt Switches attachment intents to call the new view methods for gallery/files.
presentation/src/main/java/com/moez/QKSMS/feature/compose/ComposeView.kt Removes request-code based API and exposes requestGallery() + requestFilePicker().
presentation/src/main/java/com/moez/QKSMS/feature/compose/ComposeActivity.kt Implements new Activity Result pickers for media, files, and contacts; removes legacy result-code branches.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread presentation/src/main/java/com/moez/QKSMS/feature/compose/ComposeActivity.kt Outdated
Comment thread presentation/src/main/java/com/moez/QKSMS/feature/compose/ComposeViewModel.kt Outdated

@octoshrimpy octoshrimpy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, this is very overdue, thank you. Structure looks right, and ComposeActivity is the only thing implementing ComposeView so the interface change doesn't ripple out. Also checked that androidx.activity resolves to 1.6.0 via appcompat, so the new contracts are available without a dependency bump.

Two changes, i think:

Finish the migration.

showContacts, startSpeechRecognition and requestCamera still use startActivityForResult, so we'd be keeping both mechanisms around forever. All three are plain StartActivityForResult cases and pickFilesWithChooser already shows the pattern. Keep the try/catch on speech recognition (launch() throws ActivityNotFoundException too), and stay on the chooser for camera rather than the TakePicture contract so people can still pick their camera app. Once those are over, onActivityResult and all five request codes can go. Bulkier PR, but the file ends up with one way of doing things.

Gallery needs the same old-device care the file picker got.

requestFilePicker is gated well, documents UI on 17, old chooser everywhere below. But PickMultipleVisualMedia runs unconditionally down to minSdk 23, and without a system photo picker, AndroidX falls back to a documents browser instead of the chooser people expect. Could it be gated the same way? Old Android support is important. Doubly so since #831 came from GrapheneOS, where the default "expected" photo picker may not be there at all.

Speaking of, does this actually fix #831? I can't confirm without an Android 17 device. Could we get a debug APK to the reporter first? If it works, link the issue so it closes on its own.


Agree with both of Copilot's reviews. On the stale comment, note that requestGallery now covers video too, which might partly handle #115. Link it if you think so as well.

Small stuff: the bare 37 could use a constant or a comment saying which OS that is. The uris.forEach block shows up three times and would collapse to a one line helper. Also the photo picker won't let people attach from a file manager or cloud app the way the old chooser did, so that warrants a release note for sure.

For picking contacts, photos, and files, use the modern API's. Additionally, Android 17 forces using GetMultipleContents rather than ACTION_PICK, so update that.
@Inhishonor

Copy link
Copy Markdown
Collaborator Author

Finish the migration.
showContacts, startSpeechRecognition and requestCamera still use startActivityForResult, so we'd be keeping both mechanisms around forever. All three are plain StartActivityForResult cases and pickFilesWithChooser already shows the pattern. Keep the try/catch on speech recognition (launch() throws ActivityNotFoundException too), and stay on the chooser for camera rather than the TakePicture contract so people can still pick their camera app. Once those are over, onActivityResult and all five request codes can go. Bulkier PR, but the file ends up with one way of doing things.

We can do that later, the only reason I did it here was to get the Pick any file option working on Android 17.

Gallery needs the same old-device care the file picker got.
requestFilePicker is gated well, documents UI on 17, old chooser everywhere below. But PickMultipleVisualMedia runs unconditionally down to minSdk 23, and without a system photo picker, AndroidX falls back to a documents browser instead of the chooser people expect. Could it be gated the same way? Old Android support is important. Doubly so since #831 came from GrapheneOS, where the default "expected" photo picker may not be there at all.

The reason I only care about newer versions, is that older versions aren't forced to use Document UI, which is pretty sad. #262

Speaking of, does this actually fix #831? I can't confirm without an Android 17 device. Could we get a debug APK to the reporter first? If it works, link the issue so it closes on its own.

It fixes it, I confirmed it.

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.

🐞 [BUG] Attaching 'any file' no longer works on Android 17 - GrapheneOS

3 participants