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

Add prop for App ID #30

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add prop for App ID #30

wants to merge 6 commits into from

Conversation

nosecreek
Copy link

Although the app ID isn't always required, in some cases it is needed. For example, if using the drive.file scope, the picker won't work properly without the App ID. This pull request adds the option to specify the app ID.

@wayneschuller
Copy link

Thank you for this pull request!!!!!

Setting the App ID is so important if you want to use drive.file scope.

And drive.file scope is the BEST most minimal scope!

After many hours of frustration this branch saved my project.

Could this pull request please be merged?

I would be happy to write some documentation on this.

As it stands this node package will use drive.readonly scope by default. However if you have pre-obtained a token with only drive.file scope, then it will NOT ask for drive.readonly - which is excellent!

wayneschuller added a commit to wayneschuller/strengthjourneys that referenced this pull request Jan 1, 2023
Relies on branched version of google-drive-file-picker
Jose-cd/React-google-drive-picker#30
src/typeDefs.ts Outdated
@@ -48,6 +48,7 @@ type ViewIdOptions =
export type PickerConfiguration = {
clientId: string
developerKey: string
AppId?: string
Copy link
Owner

@Jose-cd Jose-cd Jan 6, 2023

Choose a reason for hiding this comment

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

Why AppId with a capital A letter?

Copy link
Author

Choose a reason for hiding this comment

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

Looks like that must have been a typo. appId is already included on line 60, so I'll update the PR to remove this line.

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.

3 participants