Skip to content

Fix gallery intent for certain apps#1376

Merged
bmarty merged 2 commits intoelement-hq:developfrom
dkanada:patch-1
Sep 29, 2020
Merged

Fix gallery intent for certain apps#1376
bmarty merged 2 commits intoelement-hq:developfrom
dkanada:patch-1

Conversation

@dkanada
Copy link
Copy Markdown
Contributor

@dkanada dkanada commented May 17, 2020

Let me know if you'd be interested in merging this and I'll add the rest of the required items. You can see this ancient link for the reason I made the change. Lots of gallery apps don't like the open document intent and won't show up properly.

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Jun 3, 2020

@bmarty do you have any thoughts about this change? I'll close it if you want to keep the current intent but it seems like the other one might have better support to me.

@dkanada dkanada changed the base branch from master to develop July 2, 2020 05:47
@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Jul 2, 2020

You can test this change out with the Camera Roll app on F-Droid, which will only show up in the selection menu with the intent I added here.

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Jul 7, 2020

In another pull request to Riot, I had applied looping to video content in the image view fragment but it was rejected. I still really enjoy looping video content so would an option in the settings be sufficient to apply that change?

@MattDiesel
Copy link
Copy Markdown

@dkanada this sounds like it could be a possible fix for #1413 ? Possibly with the addition of FLAG_GRANT_READ_URI_PERMISSION as mentioned in this ucrop bug report: Yalantis/uCrop#669

Or just ignore me if I'm talking nonsense.

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Jul 28, 2020

I don't believe this would resolve that issue, since it's an internal activity.

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Aug 26, 2020

@bmarty if possible, I would also like to address the inconsistent capitalization of strings in this pull request. There seems to be no pattern between olm version, Terms & conditions, Help & About, Advanced settings, and similar strings. There is also inconsistent usage of punctuation for descriptions and unexplained newlines. I prefer title casing but sentence casing would also be fine.

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Aug 27, 2020

It's good to know my attempt to build yesterday wasn't a fluke and the CI is also unable to find the OLM library.

@bmarty
Copy link
Copy Markdown
Member

bmarty commented Sep 3, 2020

According to https://developer.android.com/guide/topics/providers/document-provider and specifically

image

it seems that you are right and it's better to use ACTION_GET_CONTENT in our use case.

@onurays WDYT?

@bmarty
Copy link
Copy Markdown
Member

bmarty commented Sep 3, 2020

@bmarty if possible, I would also like to address the inconsistent capitalization of strings in this pull request. There seems to be no pattern between olm version, Terms & conditions, Help & About, Advanced settings, and similar strings. There is also inconsistent usage of punctuation for descriptions and unexplained newlines. I prefer title casing but sentence casing would also be fine.

Yes, feel free to open a dedicated PR to improve this. Let's choose title casing, so for instance the new string will be Terms & Conditions

I do not know what's best practice for punctuation in setting summaries though. Maybe better to always end sentences with .

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Sep 3, 2020

That would explain why my gallery app only supports the latter option. I'm not sure many third party galleries would include an intent filter for long term, persistent access to documents. I'll open a new PR for the casing when I have some free time.

@onurays
Copy link
Copy Markdown
Contributor

onurays commented Sep 4, 2020

I just tested with ACTION_GET_CONTENT and it is working well. And now we can see other providers like Google Photos. Thanks for the PR, we can quickly test and merge after improvements on other file types.

WhatsApp Image 2020-09-04 at 14 49 39

@bmarty
Copy link
Copy Markdown
Member

bmarty commented Sep 4, 2020

@dkanada thanks for the rebase. Is this change also applicable to the 3 other Pickers?

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Sep 5, 2020

I believe so. Changing it for the audio picker should display the sound recorder as an option on LineageOS for example, but I haven't actually tested anything besides the image picker.

@dkanada
Copy link
Copy Markdown
Contributor Author

dkanada commented Sep 29, 2020

Should I fix the merge conflicts?

@bmarty
Copy link
Copy Markdown
Member

bmarty commented Sep 29, 2020

I will handle the conflicts, thanks

@bmarty bmarty merged commit 2b90f13 into element-hq:develop Sep 29, 2020
bmarty added a commit that referenced this pull request Sep 29, 2020
…instead of Intent.ACTION_OPEN_DOCUMENT for other pickers
bmarty added a commit that referenced this pull request Sep 30, 2020
Finish what has been started on #1376: use Intent.ACTION_GET_CONTENT …
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.

4 participants