feat: use immich as an image provider#23155
Conversation
|
Label error. Requires exactly 1 of: changelog:.*. Found: 📱mobile. A maintainer will add the required label. |
ttmx
changed the title
(It allows you to select multiple images and only returns the first one, needs fix)
ttmx
left a comment
ttmx
left a comment
There was a problem hiding this comment.
The code is surprisingly okay, simple and short.
I left some questions on stuff that should be looked at as I am not sure on how it works.
| </activity> | ||
|
|
||
| <!-- Image picker provider activity - handles ACTION_GET_CONTENT and ACTION_PICK --> | ||
| <activity |
| android:authorities="${applicationId}.androidx-startup" | ||
| tools:node="remove" /> | ||
|
|
||
| <!-- FileProvider for sharing images with other apps --> |
| @@ -0,0 +1,7 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Am uncertain if this is the best way to share files, does this look normal?
|
|
||
| Log.d(TAG, "ImagePickerActivity started with action: $action, type: $type") | ||
|
|
||
| if ((action == Intent.ACTION_GET_CONTENT || action == Intent.ACTION_PICK)) { |
There was a problem hiding this comment.
Perhaps a bit weird to verify the intent in two different places?
|
|
||
| override fun configureFlutterEngine(flutterEngine: FlutterEngine) { | ||
| Log.d(TAG, "configureFlutterEngine() called, hasRequestedImage = $hasRequestedImage") | ||
| super.configureFlutterEngine(flutterEngine) |
There was a problem hiding this comment.
Do we need to override and manually setup configureFlutterEngine? Isn't there another place that already does this?
| onFailure = { error -> | ||
| Log.e(TAG, "Error getting images from Flutter", error) | ||
| setResult(Activity.RESULT_CANCELED) | ||
| finish() |
|
|
||
| override fun onStart() { | ||
| super.onStart() | ||
| Log.d(TAG, "onStart() called") |
There was a problem hiding this comment.
We don't need these many logs I think
| initApp().then((_) => dPrint(() => "App Init Completed")); | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
| // Initialize the image picker provider service for ACTION_GET_CONTENT handling | ||
| ref.read(imagePickerProviderServiceProvider); |
There was a problem hiding this comment.
Do we have to initialize it here? I think we should only init that service if we're actually retrieving an image through it.
| } | ||
|
|
||
| /// Gets the URI for a single asset (local, merged, or remote) | ||
| Future<String?> _getAssetUri(BaseAsset asset) async { |
There was a problem hiding this comment.
Do we not have something similar somewhere else in the app? I feel like this might be duplicated
|
|
||
| /// API for Android native to request an image from Flutter | ||
| @FlutterApi() | ||
| abstract class ImagePickerProviderApi { |
bwees
changed the title
|
There is WIP that doesn't use 100 LLM #26109 |
|
Hey Alex! This doesn't appear to be the same feature. |
|
Ah OK, reopen for tracking |
2 participants
Hi! I want to use immich to select images for twitter and reddit, but it is not doable currently as you cannot share an image from immich to a specific reply.
I'm adding the foundations for this feature by adding the ACTION_GET_CONTENT intent and basic replies to it.
The code has heavy LLM usage, so I would kindly request an extra tough code review.
Currently the code fetches the first image on your timeline when you select immich as your image provider when using the system picker to pick an image and does not allow you to select one.Currently the code uses DriftAssetSelectionTimelinePage
to allow you to select an image, but this widget allows you to select multiple images, which means only the first image gets sent to the original Intent.and can return multiple images. We do not show the user that they can only select a single image in some cases, and the interface has awkward multiple loading screens, likely from bad code.The code likely has too many debug logs which I can fix if the feature seems promising.