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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New android sharing view #1229 #1244

Merged
merged 6 commits into from
Aug 7, 2017

Conversation

jsrck
Copy link

@jsrck jsrck commented Jul 30, 2017

This pull request enables the chooser dialog requested in #1229. In contrast to the previous version it doesn't exclude the Nextcloud-App from sharing.

@jsrck jsrck mentioned this pull request Jul 30, 2017
@mario
Copy link
Contributor

mario commented Jul 30, 2017

Thanks for the contribution. Screenshots? :)

@jsrck
Copy link
Author

jsrck commented Jul 30, 2017

It looks like on first the screenshot tobiasKaminsky posted in #1229.

On my emulator (without WhatsApp/contacts) it looks like:
send

I tried it also on my smartphone with WhatsApp installed and it looks like on tobiasKaminskys screenshot. Is it possible to test the developed version from Android Studio without uninstalling version 1.4.3 on my phone? I couldn't find out how, yet.

@tobiasKaminsky
Copy link
Member

Is it possible to test the developed version from Android Studio without uninstalling version 1.4.3 on my phone

No as both apps use the same package id :-/

@tobiasKaminsky
Copy link
Member

The old implementation was filtering the nextcloud app itself.
Is this also possible with the new view?

@jsrck
Copy link
Author

jsrck commented Jul 31, 2017

The old implementation was filtering the nextcloud app itself. Is this also possible with the new view?

I tried that as described e.g. here: https://stackoverflow.com/questions/9730243/how-to-filter-specific-apps-for-action-send-intent-and-set-a-different-text-for

But the result

  1. Contains duplicates of the ES File Explorer (somehow the different titles get lost...),
  2. Doesn't include the contacts. I didn't find an other solution including the contacts...

So with excluding Nextcloud the result is:
excluded

With the version I commited:
with-nextcloud

@tobiasKaminsky
Copy link
Member

Then I vote for "not filtering nextcloud".
What do you think? @mario @AndyScherzinger

@AndyScherzinger
Copy link
Member

I also vote for "not filtering nextcloud" :)

@mario
Copy link
Contributor

mario commented Aug 4, 2017

@AndyScherzinger @tobiasKaminsky do you want to ship this with 1.5 if it passes review?

@AndyScherzinger
Copy link
Member

@mario @tobiasKaminsky I say it depends on how fast Google fixes the build tools. If it takes to long then I'd say we ship it.

Signed-off-by: Mario Danic <mario@lovelyhq.com>
@mario
Copy link
Contributor

mario commented Aug 5, 2017

👍

Hope you guys do not mind, but I've fixed the PR by disabling NC inside of it. Please test.

Unfortunately, it does not include contacts, but all other things are shown properly. We could maybe look into this for 1.5.1, if it's all all possible without resorting to a custom view?

Thanks for the awesome contribution.

Approved with PullApprove

Signed-off-by: Mario Danic <mario@lovelyhq.com>
@mario mario added this to the Nextcloud App 1.5.0 milestone Aug 7, 2017
@tobiasKaminsky
Copy link
Member

Unfortunately, it does not include contacts

This was discussed in this issue and we agreed that it is better to have contacts (and nextcloud) in it than having no contacts, but filtered nextcloud...

@mario
Copy link
Contributor

mario commented Aug 7, 2017

@tobiasKaminsky I got the impression that this is because of the duplicate entries which I fixed.

Anyway, if @AndyScherzinger feels the same feel free to revert my commits and merge as-is.

@tobiasKaminsky
Copy link
Member

Right as said in #1244 (comment)

Doesn't include the contacts. I didn't find an other solution including the contacts...

@AndyScherzinger revert to get the contacts back again?

@AndyScherzinger
Copy link
Member

@tobiasKaminsky @mario I'd say revert if this brings back the contacts and if sharing with contacts actually works :)

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Aug 7, 2017

👍
Reverted and merged master.

Approved with PullApprove

@mario
Copy link
Contributor

mario commented Aug 7, 2017 via email

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Aug 7, 2017

Waiting for codacy/drone ;-)

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