Skip to content

Changed from custom file picker implementation to SAF#2479

Merged
jesmrec merged 4 commits into
owncloud:masterfrom
shashvat-kedia:Fix-2436-New
May 23, 2019
Merged

Changed from custom file picker implementation to SAF#2479
jesmrec merged 4 commits into
owncloud:masterfrom
shashvat-kedia:Fix-2436-New

Conversation

@shashvat-kedia

@shashvat-kedia shashvat-kedia commented Mar 10, 2019

Copy link
Copy Markdown
Contributor

@shashvat-kedia

shashvat-kedia commented Mar 10, 2019

Copy link
Copy Markdown
Contributor Author

@jesmrec @davigonz I have changed to SAF implementation and have removed the code corresponding to previous functionality(select all, select-inverse, sorting etc.).

@davigonz davigonz requested a review from abelgardep March 11, 2019 08:16
@davigonz davigonz added this to the 2.11.0 milestone Mar 11, 2019
Comment thread owncloudApp/src/main/res/layout/list_fragment.xml
@jesmrec jesmrec removed this from the 2.11.0 milestone Mar 11, 2019
@shashvat-kedia

shashvat-kedia commented Mar 22, 2019

Copy link
Copy Markdown
Contributor Author

@jesmrec @abelgardep I have made the changes.

@shashvat-kedia

Copy link
Copy Markdown
Contributor Author

@abelgardep @jesmrec Are there any more changes required in this? If not then I think this is ready for QA.

UploadFilesActivity.startUploadActivityForResult(getActivity(), ((FileActivity) getActivity())
.getAccount(), FileDisplayActivity.REQUEST_CODE__SELECT_FILES_FROM_FILE_SYSTEM);
Intent action = new Intent(Intent.ACTION_GET_CONTENT);
action = action.setType("*/*").addCategory(Intent.CATEGORY_OPENABLE);

@davigonz davigonz May 16, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please save */* in a constant?

@davigonz

Copy link
Copy Markdown
Contributor

Are there any more changes required in this? If not then I think this is ready for QA.

Just a minor change and we can move it forward.

@shashvat-kedia

Copy link
Copy Markdown
Contributor Author

@davigonz I have made the changes.

public boolean onTouch(View v, MotionEvent event) {
Intent action = new Intent(Intent.ACTION_GET_CONTENT);
action = action.setType("*/*").addCategory(Intent.CATEGORY_OPENABLE);
action = action.setType(getString(R.string.all_files_saf_filter)).addCategory(Intent.CATEGORY_OPENABLE);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I meant a constant at the top of this class, we usually include strings in owncloudApp/src/main/res/values/strings.xml when some translations are needed and this is not the case.

@shashvat-kedia

Copy link
Copy Markdown
Contributor Author

@davigonz I have reverted the previous change and have done the new one.


private static String DIALOG_CREATE_FOLDER = "DIALOG_CREATE_FOLDER";

private final String ALL_FILES_SAF_REGEX = "*/*";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👏

@davigonz

Copy link
Copy Markdown
Contributor

Code approved @SD1998 , ready to test @jesmrec

@jesmrec

jesmrec commented May 22, 2019

Copy link
Copy Markdown
Member

Let's QA this

@jesmrec jesmrec self-assigned this May 22, 2019
@jesmrec

jesmrec commented May 22, 2019

Copy link
Copy Markdown
Member

Everything correct, more simplified logic and more versatility and integration with the other apps in the device. Approved on my side.

@SD1998 could you rebase the branch to merge?

@jesmrec jesmrec self-requested a review May 22, 2019 15:22
@shashvat-kedia

Copy link
Copy Markdown
Contributor Author

@jesmrec Yeah surely.

@shashvat-kedia

Copy link
Copy Markdown
Contributor Author

@jesmrec I have done the rebase.

@jesmrec

jesmrec commented May 23, 2019

Copy link
Copy Markdown
Member

We have merged another PR, so we need here another rebase. Sorry for the inconvenience @SD1998

@shashvat-kedia

Copy link
Copy Markdown
Contributor Author

@jesmrec Not an issue. I have done the rebase.

@jesmrec jesmrec merged commit c6e2418 into owncloud:master May 23, 2019
@shashvat-kedia shashvat-kedia deleted the Fix-2436-New branch May 23, 2019 17:33
@jesmrec

jesmrec commented May 24, 2019

Copy link
Copy Markdown
Member

Thanks!!

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