Skip to content

Iteration 2 for file detail & new sharing #2573

Merged
AndyScherzinger merged 75 commits intomasterfrom
fileDetails
May 29, 2018
Merged

Iteration 2 for file detail & new sharing #2573
AndyScherzinger merged 75 commits intomasterfrom
fileDetails

Conversation

@AndyScherzinger
Copy link
Member

@AndyScherzinger AndyScherzinger commented May 11, 2018

Iteration 1 can be found here: #2533 ☑️

WiP for #2485 and collab with @tobiasKaminsky

  • use butterknife for view binding
  • remove file type icon for non-preview-able files
  • support editing/deletion of existing sharees
  • support for expiration date
  • support adding new/further sharees
  • support sharing via link
  • add fetching of sharee avatars
  • fallback theming for checkable menu items
  • FIX #2573 breaks account change #2610
  • replace ListViewwith RecyclerView

Open for iteration 3/4

  • implement actual email share type menu options (behaves as is, so atm like a user share type)
  • remove old/legacy ShareActivity and launch FileDetailFragment with sharing tab active
  • MVP the sharing part

@AndyScherzinger AndyScherzinger changed the title Iteration 2 for File detail & new sharing Iteration 2 for file detail & new sharing May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@nextcloud nextcloud deleted a comment May 11, 2018
@AndyScherzinger
Copy link
Member Author

All DONE and rebased, ready for final approval/review @tobiasKaminsky @mario @ardevd

ArbitraryDataProvider arbitraryDataProvider = new ArbitraryDataProvider(mContext.getContentResolver());

String eTag = arbitraryDataProvider.getValue(accountName, ThumbnailsCacheManager.AVATAR);
String avatarKey = "a_" + mUserId + "_" + mServerName + "_" + eTag;
Copy link
Contributor

Choose a reason for hiding this comment

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

Because I don't and don't wanna know what "a" is, please change to "avatar".

Copy link
Member

Choose a reason for hiding this comment

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

This is the key for our cache:
a_: avatar
t_: thumbnail
r_: resizedImage

As the key is changing for avatar, we can rename it, but then it is not consistent to the other.
Hopefully we/I find the time to change to glide, then this all will change again…

Copy link
Member Author

Choose a reason for hiding this comment

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

so leave it as-is then for now?

if (avatar != null && !TextUtils.isEmpty(newETag)) {
avatar = handlePNG(avatar, px, px);
String newImageKey = "a_" + username + "_" + eTag;
String newImageKey = "a_" + mUserId + "_" + mServerName + "_" + newETag;
Copy link
Contributor

Choose a reason for hiding this comment

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

avatar_ instead of a_.


// SHARE FILE
private void filterShareFile(List<Integer> toShow, List<Integer> toHide, OCCapability capability) {
boolean shareViaLinkAllowed = (mContext != null &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Useless variable. I already removed them in a private branch, but please remove them as they bring nothing. Just use their content in the one place where needed. I know it's not related to this PR, but you touch the file so :P

Copy link
Member Author

Choose a reason for hiding this comment

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

which variable?

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via 84401e7

private void filterShareFile(List<Integer> toShow, List<Integer> toHide, OCCapability capability) {
boolean shareViaLinkAllowed = (mContext != null &&
mContext.getResources().getBoolean(R.bool.share_via_link_feature));
boolean shareWithUsersAllowed = (mContext != null &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via 84401e7

mContext.getResources().getBoolean(R.bool.share_with_users_feature));

OCCapability capability = mComponentsGetter.getStorageManager().getCapability(mAccount.name);
boolean shareApiEnabled = capability != null &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via 84401e7

// SEE DETAILS
if (!isSingleFile()) {
private void filterDetails(List<Integer> toShow, List<Integer> toHide) {
if (!isSingleSelection()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Invert.

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via 84401e7


// Kept available offline
private void filterKeepAvailableOffline(List<Integer> toShow, List<Integer> toHide, boolean synchronizing) {
if (!allFiles() || synchronizing || allKeptAvailableOffline()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

And again.

Copy link
Member Author

Choose a reason for hiding this comment

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

kept since it'll end up with

if (allFiles() || !synchronizing || !allKeptAvailableOffline()) {


// Not kept available offline
private void filterDontKeepAvailableOffline(List<Integer> toShow, List<Integer> toHide, boolean synchronizing) {
if (!allFiles() || synchronizing || allNotKeptAvailableOffline()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

And everywhere else :P

Copy link
Member Author

Choose a reason for hiding this comment

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

kept since it'll end up with

if (allFiles() || !synchronizing || !allKeptAvailableOffline()) {

}

private void filterDeselectAll(List<Integer> toShow, List<Integer> toHide, boolean inSingleFileFragment) {
if (!inSingleFileFragment) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Invert again. Positive first.

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via 84401e7

private static final String ARG_ACCOUNT = "ACCOUNT";
private static final String ARG_ACTIVE_TAB = "TAB";

@Nullable @BindView(R.id.fdProgressBlock)
Copy link
Contributor

Choose a reason for hiding this comment

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

Kill the prefix pls.

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via c3209d8

@BindView(R.id.searchView)
SearchView searchView;

@BindView(R.id.fdshareUsersList)
Copy link
Contributor

Choose a reason for hiding this comment

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

fd again.

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via c3209d8



/// BEWARE: next methods will failed with NullPointerException if called before onCreateView() finishes
// BEWARE: next methods will failed with NullPointerException if called before onCreateView() finishes
Copy link
Contributor

Choose a reason for hiding this comment

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

"will fail" instead of "will failed".

also "next" -> "the following"

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via c3209d8

searchView.setOnQueryTextListener(new SearchView.OnQueryTextListener() {
@Override
public boolean onQueryTextSubmit(String query) {
// return true to prevent the query is processed to be queried;
Copy link
Contributor

Choose a reason for hiding this comment

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

Uh. Can you rephrase this entire sentence? xD

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via c3209d8

String eTag = arbitraryDataProvider.getValue(account, ThumbnailsCacheManager.AVATAR);
String serverName = account.name.substring(account.name.lastIndexOf('@') + 1, account.name.length());
String eTag = arbitraryDataProvider.getValue(userId + "@" + serverName, ThumbnailsCacheManager.AVATAR);
String avatarKey = "a_" + userId + "_" + serverName + "_" + eTag;
Copy link
Contributor

Choose a reason for hiding this comment

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

"avatar_"

<size android:height="1dp" />
</shape>
</item>
</layer-list> No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

New line.

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via c3209d8

android:title="@string/common_rename"
app:showAsAction="never"
android:showAsAction="never"
android:orderInCategory="1" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove orderInCategory

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via c3209d8

<string name="tags">Tags</string>
<string name="sharee_add_failed">Adding sharee failed</string>
<string name="unsharing_failed">Unsharing failed</string>
<string name="updating_share_failed">Updateing share failed</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Updaeting -> Updating

Copy link
Member Author

Choose a reason for hiding this comment

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

DONE via c3209d8

@mario
Copy link
Contributor

mario commented May 29, 2018

👍

Decent enough :P

Approved with PullApprove

@AndyScherzinger
Copy link
Member Author

Just waiting for Drone to "succeed"...

@AndyScherzinger
Copy link
Member Author

Lint check is green, everything is green, Drone is not working right again... so merging

@jancborchardt
Copy link
Member

Yay, awesome work! :) Really looking forward to this landing in the release!

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