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

Display copy icon and adding send action to the link menu #2730

Merged
merged 8 commits into from
Jun 20, 2018

Conversation

AndyScherzinger
Copy link
Member

Fix #2705

device-2018-06-19-165059

device-2018-06-19-165115

Beware that I changed the behavior a bit as-in copy to clipboard is now a Toast, not a Snackbar. I checked the behavior for the Android OS and typical (Google) apps and they all show a Toast for this.

@jancborchardt I did change the layout a bit (see screenshots) since aligning the copy icon next to the 3-dot menu seemed strange since shared folders also have the "edit option" for link-share so I thought it be better to add a small copy-icon-badge to the text to indicate it can be clicked and moved the "send link" functionality to the link's menu (see screenshot). Is this fine with you?

Also @tobiasKaminsky @mario please review ❤️

Furthermore I think this should be put into 3.2.1 so I vote to have a backport 🎉

@codecov
Copy link

codecov bot commented Jun 19, 2018

Codecov Report

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

@@            Coverage Diff            @@
##           master   #2730      +/-   ##
=========================================
+ Coverage    6.49%   6.49%   +<.01%     
=========================================
  Files         287     288       +1     
  Lines       29058   29070      +12     
  Branches     4234    4235       +1     
=========================================
+ Hits         1888    1889       +1     
- Misses      26886   26897      +11     
  Partials      284     284
Impacted Files Coverage Δ
...java/com/owncloud/android/utils/ClipboardUtil.java 0% <0%> (ø)
...cloud/android/ui/activity/FileDisplayActivity.java 0% <0%> (ø) ⬆️
...android/ui/fragment/FileDetailSharingFragment.java 0% <0%> (ø) ⬆️
...d/android/ui/activity/CopyToClipboardActivity.java 0% <0%> (ø) ⬆️
.../third_parties/daveKoeller/AlphanumComparator.java 82.75% <0%> (+1.14%) ⬆️

@mario
Copy link
Contributor

mario commented Jun 19, 2018

👍
Minor codacy stuff.

Approved with PullApprove

@nextcloud nextcloud deleted a comment Jun 19, 2018
@nextcloud nextcloud deleted a comment Jun 19, 2018
@nextcloud nextcloud deleted a comment Jun 19, 2018
@AndyScherzinger
Copy link
Member Author

Thanks @mario ❤️ Just pushed the fix for codacy :)


Toast.makeText(activity, R.string.clipboard_text_copied, Toast.LENGTH_SHORT).show();
} catch (Exception e) {
Toast.makeText(activity, R.string.clipboard_uxexpected_error, Toast.LENGTH_SHORT).show();
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the typo, though it is not by you ;-)

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Jun 20, 2018

👍
If "share link" is clickable, the copy icon is, for me, a bit too narrow to it.

Approved with PullApprove

@AndyScherzinger
Copy link
Member Author

Will fix typo and whitespace later today :)

Thank you all for the feedback and review 👍

@jancborchardt
Copy link
Member

Looks good! I would also only have the feedback of the copy icon needing more whitespace around :)

@AndyScherzinger
Copy link
Member Author

@jancborchardt @tobiasKaminsky like this?
device-2018-06-20-112312

@jancborchardt
Copy link
Member

Looks quite similar to the first screenshot it seems. The icon needs to be moved a bit down (verticaly centered with the checkboxes and text) and a bit more to the right.

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Jun 20, 2018

Ah, got it. I always tried to have it as some kind of a badge while you folks want it to me more like a menu icon (size, whitespace, etc.):
device-2018-06-20-114432

@jancborchardt
Copy link
Member

Yesss, perfect! :)

@tobiasKaminsky tobiasKaminsky merged commit db0b0cb into master Jun 20, 2018
@tobiasKaminsky tobiasKaminsky deleted the shareLinkHandling branch June 20, 2018 11:48
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