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

Improve share link handling #2935

Merged
merged 5 commits into from
Sep 4, 2018
Merged

Conversation

tilosp
Copy link
Member

@tilosp tilosp commented Aug 29, 2018

Resolves #2742

@codecov
Copy link

codecov bot commented Aug 29, 2018

Codecov Report

Merging #2935 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #2935      +/-   ##
=========================================
+ Coverage    6.52%   6.52%   +<.01%     
=========================================
  Files         301     301              
  Lines       29734   29755      +21     
  Branches     4296    4298       +2     
=========================================
+ Hits         1941    1943       +2     
- Misses      27501   27521      +20     
+ Partials      292     291       -1
Impacted Files Coverage Δ
...cloud/android/ui/activity/FileDisplayActivity.java 0% <0%> (ø) ⬆️
...java/com/owncloud/android/utils/ClipboardUtil.java 0% <0%> (ø) ⬆️
...android/ui/fragment/FileDetailSharingFragment.java 0% <0%> (ø) ⬆️
.../third_parties/daveKoeller/AlphanumComparator.java 83.33% <0%> (+2.38%) ⬆️

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Aug 30, 2018

👍 code is fine with me and also successfully tested on OP3T. I added some theming code for the snackbar's action. cc @tobiasKaminsky for review and approval. I vote to sneak this into 3.3.0 :)

Approved with PullApprove

@nextcloud nextcloud deleted a comment Aug 30, 2018
@AndyScherzinger
Copy link
Member

@tobiasKaminsky rebased to latest master ❗

@AndyScherzinger AndyScherzinger mentioned this pull request Aug 30, 2018
58 tasks
@nextcloud nextcloud deleted a comment Aug 30, 2018
@nextcloud nextcloud deleted a comment Sep 2, 2018
@nextcloud nextcloud deleted a comment Sep 3, 2018
@tobiasKaminsky
Copy link
Member

  • open up file detail view
  • click on checkmark for "share link", see snackbar with "share" -> ✔️
  • click on "clipboard icon", get toast -> ✔️
  • open overflow menu, click "send link", see again snackbar ❌ as the user then needs again to click on "share" within snackbar. Here the "share dialog" should immediately be shown
    (also it seems as a new network call is done, this is not necessary, or?)

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Sep 3, 2018

  • open overflow menu, click "send link", see again snackbar ❌ as the user then needs again to click on "share" within snackbar. Here the "share dialog" should immediately be shown
    (also it seems as a new network call is done, this is not necessary, or?)

I'll fix that, it is a bug always been there

@AndyScherzinger
Copy link
Member

@tobiasKaminsky thanks for the thorough testing 👍 I fixed the issue you found via 1a565d2

@nextcloud nextcloud deleted a comment Sep 3, 2018
@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Sep 4, 2018

👍 Thanks for the fix, working now as expected

@tilosp thanks again for this PR 🎉
It was nice to meet you at Conf!

Approved with PullApprove

@nextcloud nextcloud deleted a comment Sep 4, 2018
@nextcloud-android-bot
Copy link
Collaborator

Lint

TypeMasterPR
Warnings9393
Errors

FindBugs (new)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings166
Experimental Warnings4
Internationalization Warnings12
Malicious code vulnerability Warnings9
Multithreaded correctness Warnings9
Performance Warnings156
Security Warnings163
Dodgy code Warnings135
Total689

FindBugs (master)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings166
Experimental Warnings4
Internationalization Warnings12
Malicious code vulnerability Warnings9
Multithreaded correctness Warnings9
Performance Warnings156
Security Warnings163
Dodgy code Warnings135
Total689

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants