-
Notifications
You must be signed in to change notification settings - Fork 42
added limitations section to flutter sdk documentation #597
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughDocumentation updates to the Flutter SDK: redirect URIs and examples switched to the com.kinde.myapp scheme; iOS CFBundleURLSchemes format adjusted; Dart and platform setup examples aligned; added a mobile WebView authentication Limitations note; fixed quoting and code-block formatting; removed front-matter metadata attributes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx (1)
680-683: Limitations section is clear and helpful; consider minor wording refinement.The new Limitations section effectively explains the WebView authentication constraint, including the technical reason (ephemeral sessions and separate storage) and provides a support path. For conciseness, consider replacing "are not accessible" with "are inaccessible" on line 682.
-Authentication tokens are stored in native secure storage (Keychain/KeyStore) and are not accessible to WebView contexts, which maintain separate cookie storage. +Authentication tokens are stored in native secure storage (Keychain/KeyStore) and are inaccessible to WebView contexts, which maintain separate cookie storage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx(7 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
📚 Learning: 2024-10-22T06:12:51.559Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
Applied to files:
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
📚 Learning: 2024-10-22T06:13:23.808Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:13:23.808Z
Learning: In the Kinde documentation, links may use `localhost` URLs that are fixed to relative paths, and the docs base URL is not `kinde.com`.
Applied to files:
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
🪛 LanguageTool
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
[style] ~682-~682: Consider using “inaccessible” to avoid wordiness.
Context: ...ure storage (Keychain/KeyStore) and are not accessible to WebView contexts, which maintain sep...
(NOT_ABLE_PREMIUM)
🔇 Additional comments (4)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx (4)
86-87: Concrete example aids clarity.Providing
com.kinde.myappas a concrete scheme example alongside the generic template is helpful for developers. Verify this scheme is used consistently in all code examples throughout the document.
127-127: Clarify scheme naming inconsistency.Line 127 correctly removes
://from the plist value. However, the note on Line 133 referencescom.kinde.appwhile the concrete examples (lines 86-87, 149-150) usecom.kinde.myapp. This inconsistency may confuse developers about which scheme name to use. Verify whether the note should referencecom.kinde.myappto align with the provided examples, or clarify the distinction if intentional.Also applies to: 133-133
149-150: Code example consistency maintained.The concrete scheme examples in the Dart code block align well with the environment variable examples. This consistency aids developer understanding.
826-826: API reference documentation completeness improved.The updates on lines 826, 842, and 1029 complete incomplete parameter documentation in the API reference section:
- Line 826: Clarifies parameter type syntax
- Line 842: Completes parameter name for
getPermission- Line 1029: Completes type annotation for
getStringFlagThese fix inconsistencies in the parameter documentation blocks.
Also applies to: 842-842, 1029-1029
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx (1)
707-709: Well-articulated Limitations section addressing known WebView behavior.The new section clearly documents the authentication limitation, explains the root cause (ephemeral sessions, separate cookie storage), and provides a support contact path. This is valuable for developer transparency.
The static analysis tool suggests a minor wording improvement for conciseness. Consider:
-Authentication tokens are stored in native secure storage (Keychain/KeyStore) and are not accessible to WebView contexts +Authentication tokens are stored in native secure storage (Keychain/KeyStore) and are inaccessible to WebView contexts
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-22T06:12:51.559Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
Applied to files:
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
📚 Learning: 2024-10-22T06:13:23.808Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:13:23.808Z
Learning: In the Kinde documentation, links may use `localhost` URLs that are fixed to relative paths, and the docs base URL is not `kinde.com`.
Applied to files:
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
🪛 LanguageTool
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
[style] ~709-~709: Consider using “inaccessible” to avoid wordiness.
Context: ...ure storage (Keychain/KeyStore) and are not accessible to WebView contexts, which maintain sep...
(NOT_ABLE_PREMIUM)
🔇 Additional comments (3)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx (3)
113-114: Well-updated example scheme references.The environment variable examples now show concrete
com.kinde.myappURIs, providing clearer guidance alongside the placeholder template.
154-154: Correct iOS CFBundleURLSchemes format.Removing the
://suffix aligns with proper iOS plist configuration where the scheme name alone (without protocol separators) is expected.
853-853: Verify API reference type annotation syntax changes.The changes at these lines appear to adjust quote usage in type documentation (e.g.,
TokenType "access_token"at line 853,"string"literals at lines 869 and 1056). Verify these are corrections to pre-existing issues and not new inconsistencies introduced.Please confirm:
- Were these changes fixing missing or malformed quotes in the original documentation?
- Should the TypeScript-style type notation (
"access_token" | "id_token") be replaced with Dart-idiomatic type hints?If available, please share the diff of the original vs. current state for these line ranges.
Also applies to: 869-869, 1056-1056
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx (1)
707-709: Well-written Limitations section that clearly documents the known constraint.The new section effectively explains the WebView authentication limitation, the underlying cause (ephemeral sessions and separate secure storage), and provides a clear path for developer support. This will help users understand the constraint and set realistic expectations.
As a minor optional refinement, consider using "inaccessible" instead of "are not accessible" for conciseness (line 709).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx(8 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-10-22T06:12:51.559Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:12:51.559Z
Learning: The Kinde documentation's base URL is not `kinde.com`, so links to the documentation should use the correct base URL.
Applied to files:
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
📚 Learning: 2024-10-22T06:13:23.808Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 226
File: src/content/docs/developer-tools/sdks/backend/nextjs-sdk.mdx:0-0
Timestamp: 2024-10-22T06:13:23.808Z
Learning: In the Kinde documentation, links may use `localhost` URLs that are fixed to relative paths, and the docs base URL is not `kinde.com`.
Applied to files:
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
📚 Learning: 2024-11-12T06:00:08.396Z
Learnt from: marcosmartini
Repo: kinde-oss/documentation PR: 253
File: src/content/docs/properties/work-with-properties/property-groups.mdx:13-13
Timestamp: 2024-11-12T06:00:08.396Z
Learning: In the project documentation, the `/kinde-apis/management` and `/kinde-apis/frontend` pages use a third-party API reference and client that loads a Single Page Application (SPA). Therefore, links to sections within these pages are valid, even if they appear to point to non-existent sections in static analysis, and should not be flagged.
Applied to files:
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
🪛 LanguageTool
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx
[style] ~709-~709: Consider using “inaccessible” to avoid wordiness.
Context: ...ure storage (Keychain/KeyStore) and are not accessible to WebView contexts, which maintain sep...
(NOT_ABLE_PREMIUM)
🔇 Additional comments (4)
src/content/docs/developer-tools/sdks/native/flutter-sdk.mdx (4)
160-160: Past issues resolved successfully.The critical and major issues flagged in previous reviews have been addressed:
- Line 160: Scheme reference now correctly shows
com.kinde.myappwithout the broken hyperlink.- Lines 176–177: Dart code now has properly quoted redirect URI strings.
Also applies to: 176-177
113-114: Scheme examples are properly aligned.The concrete environment variable examples correctly use
com.kinde.myappscheme, which aligns with the documentation updates and provides clear guidance after the generic placeholder examples.
154-154: iOS plist format is correct.The CFBundleURLSchemes value now correctly omits the trailing
://, which is the proper plist syntax. The scheme value will be used with://when constructing redirect URIs elsewhere in the app.
853-853: Clarify API documentation syntax for tokenKey parameter.Line 853 shows
tokenKey?: TokenType "access_token" | "id_token"which mixes a type name with a union syntax in an unusual way. Please verify this is the intended format or if it should be simplified to one of:
tokenKey?: "access_token" | "id_token"(union of literals)tokenKey?: TokenType(reference to the type definition)
|
Everything looks great to me. It's okay to be merged. |
tamalchowdhury
left a comment
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.
Looks all good.
##Add WebView authentication limitation to Flutter SDK docs
#Summary:
Documents known limitation where Flutter SDK users must authenticate separately in WebViews on mobile platforms (iOS/Android).
#Changes:
Added ## Limitations section to Flutter SDK documentation
Explains ephemeral session behavior and native secure storage architecture
Provides contact path for alternative integration patterns
Summary by CodeRabbit
Documentation
Bug Fixes