-
Notifications
You must be signed in to change notification settings - Fork 205
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 the option to choose additional scopes at sign-in #30
Conversation
Very useful functionality. Today I ran into this problem on the project. |
Thanks for the PR. Please note that the separation of |
Any idea on the timeline to conclude if this change will be approved or not? |
@petea It seems that the use cases for this functionality have been documented in #23. I would add my predicament as an SDK developer is that I don't know whether to release an update with a breaking change based on the new 2-step add scope flow, or whether this PR will get approved and I can release an update where the upgrade to v6 will be transparent to users of my SDK (obviously strongly prefer the latter, but overall just want to know what the verdict will be). Thanks! |
any news? |
@petea To weigh in, it's also paramount for us that we don't prompt twice give the first screen of our app uses the calendar permission. I think a lot of people have outlined their concerns and use cases in the issue below. It provides a poor user experience and also means we won't be upgrading to this library in the meantime. I'm not entirely sure this library should be opinonated on UX flow. If it's required that apps request scope at time of using that feature, surely that is up to the individual use cases. Those apps which require scope extensions immediately are penalised. |
Copying PR from Anton Kozllovskyi google#30
@AntonKozlovskyi thanks again for your contribution! We now plan to ship this with the 6.2 release. See my update on #23. |
* fix calling callback on addScopes * Add possibility to choose scopes in time of sign in * fixes due review * Complete merge from main Co-authored-by: Peter Andrews <[email protected]>
@@ -192,6 +192,20 @@ - (void)signInWithConfiguration:(GIDConfiguration *)configuration | |||
[self signInWithOptions:options]; | |||
} | |||
|
|||
- (void)signInWithConfiguration:(GIDConfiguration *)configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
000IOEC. P [email protected] omreceipt.here
No description provided.