iOS : add mode option #345

RichardFevrier · 2020-09-04T15:31:17Z

No description provided.

RichardFevrier · 2020-09-04T15:43:00Z

Related to #335

RichardFevrier · 2020-10-01T13:50:20Z

Hello @vonovak 👋 I know you are probably buzy but can you take a moment to look at our PRs please 🙏

RichardFevrier · 2020-10-01T14:36:40Z

This will allow to close this and this by the way 🙂

vonovak

hi! Thanks for the PR! I have just a few questions, and some suggestions, can you please address those? Thanks!

vonovak · 2020-10-06T08:50:36Z

index.d.ts

      options: DocumentPickerOptions<OS>
    ): Promise<DocumentPickerResponse[]>;
    static isCancel<IError extends { code?: string }>(err?: IError): boolean;
+    static releaseSecureAccess(uri: string): void;


question: would it not be more convenient for this method to accept an array of uris?

Good suggestion!

vonovak · 2020-10-06T08:52:25Z

index.js

  return RNDocumentPicker.pick(opts);
 }

+function releaseSecureAccess(uri) {


this fn seems a little redundant, perhaps it'd make sense to call it inline in static releaseSecureAccess? Also, question, what is the return value here? releaseSecureAccess is defined as returning void

You're right it's a mistake, I will fix this.

vonovak · 2020-10-06T08:57:00Z

ios/RNDocumentPicker/RNDocumentPicker.m

+
        if (!fileError) {
-            [result setValue:newURL.absoluteString forKey:FIELD_URI];
+            [result setValue:((mode == UIDocumentPickerModeOpen) ? url : newURL).absoluteString forKey:FIELD_URI];


just wondering, why is this needed? thanks!

Because sometimes those two urls where a little bit different during my tests and I wasn't able to save documents with the new one.

vonovak · 2020-10-07T08:00:41Z

ios/RNDocumentPicker/RNDocumentPicker.m

    NSMutableArray *composeResolvers;
    NSMutableArray *composeRejecters;
-    NSString* copyDestination;
+    NSMutableArray *urls;


question: maybe I'm missing something; where is this array allocated?

My bad, it's fixed now.

vonovak · 2020-10-07T11:37:22Z

@RichardFevrier thanks, this looks good to me; I'll just test this out on a device some time later and merge then.

FYI it'd be easier for review if you didn't force-push everything into one commit but instead kept adding new commits after each review, that way reviewer can see the changes done in between the reviews more easily. Thanks! 👍

RichardFevrier · 2020-10-07T12:19:13Z

@vonovak I'll take your advice.
And thanks for taking the time to review my work 🙂

vonovak · 2020-12-15T00:58:30Z

@RichardFevrier sorry for taking so long, I'm just taking a look at the last commit you made: 4b4fb06 -

I'm always cautious of modifying a collection while looping over it (although I don't know the specifics of objective-C):

    for (NSString *uri in uris) {
        NSURL *url = [NSURL URLWithString:uri];
        [url stopAccessingSecurityScopedResource];
        for (NSURL *url in urls) { //<--- enumeration
            if ([url.absoluteString isEqual:uri]) {
                [url stopAccessingSecurityScopedResource];
                [urls removeObject:url]; //<--- modification
                break;
            }
        }
    }

Would you please use one of the solutions from https://stackoverflow.com/questions/111866/best-way-to-remove-from-nsmutablearray-while-iterating?

Also, just wondering, what was the reason for the change? Thank you so much and sorry for taking so long!

vonovak · 2020-12-25T18:15:42Z

hello @RichardFevrier and thanks for the PR! Sorry I took so long. Would you be interested in becoming a collaborator on the repository? Thank you! 🙂

RichardFevrier · 2021-01-19T15:10:20Z

Hello @vonovak thank you for taking the time to review my work!
I would be really happy to collaborate with you guys 🙂
I have more work that’s ready to be reviewed.
I will try to do even better work next time 😅

vonovak · 2021-01-20T18:34:16Z

@RichardFevrier great! Let me add you as a collaborator; feel free to do what you see fit in the repo - you'll be able to create branches directly in the repo (no need for forks), you can close issues, improve docs, work on whatever you want, as much or as little as you want, no pressure :)

If you wanna discuss something, we can do that here or reach out to me on twitter. Thanks!

RichardFevrier · 2021-01-22T15:04:04Z

Great, thanks! I look forward to working on it 😌

* upstream/master: (22 commits) 5.0.2 fix: copyTo on android to put the file in a uniquely named dir (react-native-documents#397) 5.0.1 Fix copyFile on Android. (react-native-documents#392) chore: document RN blob support 5.0.0 support only android >=5 (API 21) (react-native-documents#386) [Android][Google Drive] Download file from google drive then cache in local storage (react-native-documents#264) 4.3.0 [Windows] Implementing Folder Picker support (react-native-documents#380) Fix crash on iOS (react-native-documents#383) chore: deleted duplicate test case (react-native-documents#375) chore: change readme wording 4.2.0 iOS : add mode option (react-native-documents#345) chore: update comment for closing issues (react-native-documents#373) chore: update discussions in readme (react-native-documents#372) chore: introduce GH discussions (react-native-documents#371) chore: add issue actions (react-native-documents#369) chore: update provided file type shortcuts ...

RichardFevrier force-pushed the master branch 3 times, most recently from bbb7cad to 0a397ce Compare September 4, 2020 15:41

RichardFevrier force-pushed the master branch from 0a397ce to 9b7a1c3 Compare October 1, 2020 14:15

This was referenced Oct 1, 2020

FR: Select Folder Location #302

Closed

Picking a folder to save to #4

Closed

RichardFevrier force-pushed the master branch from f10c45b to 9b7a1c3 Compare October 1, 2020 14:41

vonovak reviewed Oct 6, 2020

View reviewed changes

RichardFevrier force-pushed the master branch 3 times, most recently from 23b8778 to b22f26e Compare October 6, 2020 13:38

RichardFevrier requested a review from vonovak October 6, 2020 13:44

vonovak reviewed Oct 7, 2020

View reviewed changes

iOS : add mode option

cf12ec8

RichardFevrier force-pushed the master branch from b22f26e to cf12ec8 Compare October 7, 2020 09:44

RichardFevrier requested a review from vonovak October 7, 2020 09:46

RichardFevrier and others added 2 commits October 10, 2020 14:00

iOS : fix add mode option

4b4fb06

readme: fix releaseSecureAccess signature

d328ad7

fix: do not mutate collection while enumerating

9e8dd38

vonovak approved these changes Dec 25, 2020

View reviewed changes

vonovak merged commit 0ead2a5 into react-native-documents:master Dec 25, 2020

Uh oh!

iOS : add mode option #345

iOS : add mode option #345

Uh oh!

Conversation

RichardFevrier commented Sep 4, 2020

Uh oh!

RichardFevrier commented Sep 4, 2020

Uh oh!

RichardFevrier commented Oct 1, 2020

Uh oh!

RichardFevrier commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vonovak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vonovak commented Oct 7, 2020

Uh oh!

RichardFevrier commented Oct 7, 2020

Uh oh!

vonovak commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vonovak commented Dec 25, 2020

Uh oh!

RichardFevrier commented Jan 19, 2021

Uh oh!

vonovak commented Jan 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RichardFevrier commented Jan 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RichardFevrier commented Oct 1, 2020 •

edited

Loading

vonovak commented Dec 15, 2020 •

edited

Loading

vonovak commented Jan 20, 2021 •

edited

Loading