feat: custom UI updates #28
Conversation
WalkthroughThe changes to Changes
Sequence DiagramsequenceDiagram
participant Client
participant MainLib
participant AssetManager
Client->>MainLib: getLogoUrl(orgCode)
MainLib->>AssetManager: getAssetUrl(logoPath, orgCode)
AssetManager-->>MainLib: Constructed Asset URL
MainLib-->>Client: Return Logo URL
Client->>MainLib: setKindeDesignerCustomProperties(options)
MainLib->>MainLib: Generate CSS Custom Properties
MainLib-->>Client: Return CSS Property String
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/main.ts (3)
12-16: Consider making the cache parameter configurable and adding input validationThe
getAssetUrlfunction has two potential improvements:
- The cache parameter is hardcoded, which might make cache busting difficult
- The orgCode parameter should be sanitized before being used in URL construction
Consider this implementation:
const getAssetUrl = (assetPath: string, orgCode?: OrgCode) => { + const sanitizedOrgCode = orgCode?.replace(/[^a-zA-Z0-9_]/g, ''); + const cacheKey = process.env.ASSET_CACHE_KEY || '@8973ff883c2c40e1bad198b543e12b24@'; - return `/${assetPath}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=@8973ff883c2c40e1bad198b543e12b24@`; + return `/${assetPath}?${sanitizedOrgCode ? `p_org_code=${sanitizedOrgCode}&` : ""}cache=${cacheKey}`; };
Line range hint
349-374: Consider deprecating duplicate URL getter functionsThere are two pairs of functions that return the same GUIDs:
getKindeRegisterUrlandgetKindeSignUpUrlgetKindeLoginUrlandgetKindeSignInUrlConsider marking one function from each pair as deprecated:
/** @deprecated Use getKindeRegisterUrl instead */ export const getKindeSignUpUrl = (): KindePlaceholder => registerGUID; /** @deprecated Use getKindeLoginUrl instead */ export const getKindeSignInUrl = (): KindePlaceholder => loginGUID;
375-389: Consider making orgCode optional and add JSDoc commentsThe asset URL functions could be improved in two ways:
- Consider making
orgCodeoptional to maintain flexibility- Add JSDoc comments to document the expected asset paths and usage
Example implementation:
/** * Returns the URL for the organization's logo * @param orgCode - Optional organization code (e.g., "org_123") * @returns URL to the logo asset */ export const getLogoUrl = (orgCode?: OrgCode) => { return getAssetUrl("logo", orgCode); };
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/types.ts (1)
147-155: Add type constraints for design propertiesThe
KindeDesignerCustomPropertiestype allows any string values for colors and border radii. Consider adding more specific types to ensure valid values.export type KindeDesignerCustomProperties = { - baseBackgroundColor?: string; - baseLinkColor?: string; - buttonBorderRadius?: string; - primaryButtonBackgroundColor?: string; - primaryButtonColor?: string; - cardBorderRadius?: string; - inputBorderRadius?: string; + baseBackgroundColor?: `#${string}` | `rgb(${number}, ${number}, ${number})` | `rgba(${number}, ${number}, ${number}, ${number})`; + baseLinkColor?: `#${string}` | `rgb(${number}, ${number}, ${number})` | `rgba(${number}, ${number}, ${number}, ${number})`; + buttonBorderRadius?: `${number}${'px' | 'rem' | 'em' | '%'}`; + primaryButtonBackgroundColor?: `#${string}` | `rgb(${number}, ${number}, ${number})` | `rgba(${number}, ${number}, ${number}, ${number})`; + primaryButtonColor?: `#${string}` | `rgb(${number}, ${number}, ${number})` | `rgba(${number}, ${number}, ${number}, ${number})`; + cardBorderRadius?: `${number}${'px' | 'rem' | 'em' | '%'}`; + inputBorderRadius?: `${number}${'px' | 'rem' | 'em' | '%'}`; };🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues found. Run Prettier with --write to fix.
lib/main.ts (2)
13-15: Consider making the cache parameter configurableThe cache parameter is hardcoded, which might cause issues with cache invalidation. Consider making it configurable or using a dynamic value.
-const getAssetUrl = (assetPath: string, orgCode?: OrgCode) => { - return `/${assetPath}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=@8973ff883c2c40e1bad198b543e12b24@`; +const getAssetUrl = (assetPath: string, orgCode?: OrgCode, cacheKey?: string) => { + const defaultCacheKey = "@8973ff883c2c40e1bad198b543e12b24@"; + return `/${assetPath}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=${cacheKey || defaultCacheKey}`; };🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues found. Run Prettier with --write to fix.
374-388: Add JSDoc comments for asset URL functionsThe asset URL functions lack documentation. Add JSDoc comments to explain their purpose and parameters.
+/** + * Get the URL for the organization's logo + * @param orgCode - The organization's code + * @returns The URL for the logo + */ export const getLogoUrl = (orgCode: OrgCode) => { return getAssetUrl("logo", orgCode); }; +/** + * Get the URL for the organization's dark mode logo + * @param orgCode - The organization's code + * @returns The URL for the dark mode logo + */ export const getDarkModeLogoUrl = (orgCode: OrgCode) => { return getAssetUrl("logo_dark", orgCode); };🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues found. Run Prettier with --write to fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/main.ts(3 hunks)lib/types.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Build and test
lib/types.ts
[warning] Code formatting issues found. Run Prettier with --write to fix.
lib/main.ts
[warning] Code formatting issues found. Run Prettier with --write to fix.
🔇 Additional comments (4)
lib/types.ts (2)
157-157: LGTM!The
OrgCodetype effectively ensures that organization codes follow the required format.🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues found. Run Prettier with --write to fix.
Line range hint
1-157: Fix code formatting issuesThe pipeline indicates formatting issues. Run Prettier to fix the formatting.
🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues found. Run Prettier with --write to fix.
lib/main.ts (2)
390-408: Fix the baseLinkColor template and improve the implementationThe function has several issues that were previously identified:
- Extra
$in the baseLinkColor template- Inconsistent indentation
- No validation for color format or border radius values
Please refer to the previous review comment for the suggested implementation.
🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues found. Run Prettier with --write to fix.
Line range hint
1-408: Fix code formatting issuesThe pipeline indicates formatting issues. Run Prettier to fix the formatting.
🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code formatting issues found. Run Prettier with --write to fix.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/main.ts (3)
19-21: Add input validation for assetPath parameter.While the function implementation is correct, consider adding validation for the assetPath parameter to prevent potential issues with empty strings or invalid characters.
const getAssetUrl = (assetPath: string, orgCode?: OrgCode) => { + if (!assetPath?.trim()) { + throw new Error('Asset path cannot be empty'); + } + if (!/^[\w\-./]+$/.test(assetPath)) { + throw new Error('Asset path contains invalid characters'); + } return `/${assetPath}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=@8973ff883c2c40e1bad198b543e12b24@`; };
380-394: Add JSDoc documentation for asset URL functions.Consider adding JSDoc documentation to improve code maintainability and IDE support.
+/** + * Returns the URL for the organization's logo + * @param orgCode - The organization's code + * @returns The URL string for the logo + */ export const getLogoUrl = (orgCode: OrgCode) => { return getAssetUrl("logo", orgCode); }; +/** + * Returns the URL for the organization's dark mode logo + * @param orgCode - The organization's code + * @returns The URL string for the dark mode logo + */ export const getDarkModeLogoUrl = (orgCode: OrgCode) => { return getAssetUrl("logo_dark", orgCode); };
439-447: Fix template string indentation and improve error messages.The template string has inconsistent indentation, and error messages could be more specific about which value is invalid.
- return ` - ${baseBackgroundColor ? `--kinde-designer-base-background-color: ${baseBackgroundColor};` : ""} - ${baseLinkColor ? `--kinde-designer-base-link-color: ${baseLinkColor};` : ""} + return `${baseBackgroundColor ? `--kinde-designer-base-background-color: ${baseBackgroundColor};` : ""} +${baseLinkColor ? `--kinde-designer-base-link-color: ${baseLinkColor};` : ""} // ... similar changes for other properties `.trim();Also, improve error messages:
- throw new Error("Invalid color value provided"); + throw new Error(`Invalid color format. Colors must be in hex (#RRGGBB), rgb(), or rgba() format`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/main.ts(3 hunks)lib/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/types.ts
🧰 Additional context used
📓 Learnings (1)
lib/main.ts (1)
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:348-349
Timestamp: 2025-01-10T14:04:01.690Z
Learning: In the infrastructure repository, both sets of URL functions (getKindeSignUpUrl/getKindeSignInUrl and getKindeRegisterUrl/getKindeLoginUrl) should be maintained as active, non-deprecated APIs.
🔇 Additional comments (2)
lib/main.ts (2)
7-13: LGTM! Clean import organization.The imports are well-structured and properly grouped.
Line range hint
354-378: LGTM! Implementation aligns with requirements.Both sets of URL functions are maintained as active APIs as per the retrieved learning. The implementation is clean and consistent.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/main.ts (2)
20-22: Consider making the cache parameter configurable.The cache parameter is currently hardcoded. Consider making it configurable to allow for cache busting when needed.
-const getAssetUrl = (assetPath: string, orgCode?: OrgCode) => { +const getAssetUrl = (assetPath: string, orgCode?: OrgCode, cacheKey?: string) => { - return `/${assetPath}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=@8973ff883c2c40e1bad198b543e12b24@`; + return `/${assetPath}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=${cacheKey || "@8973ff883c2c40e1bad198b543e12b24@"}`;🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code style issues found. Prettier formatting rules are not met.
435-444: Consider extracting regular expressions as constants.The regular expressions for validation could be extracted as named constants for better maintainability and reuse.
+const COLOR_REGEX = /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$|^rgb\(.*\)$|^rgba\(.*\)$/; +const BORDER_RADIUS_REGEX = /^\d+(%|px|rem|em)$/; -const isValidColor = (color: string | undefined) => - !color || - /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$|^rgb\(.*\)$|^rgba\(.*\)$/.test(color); +const isValidColor = (color: string | undefined) => + !color || COLOR_REGEX.test(color); -const isValidBorderRadius = (radius: string | undefined) => - !radius || /^\d+(%|px|rem|em)$/.test(radius); +const isValidBorderRadius = (radius: string | undefined) => + !radius || BORDER_RADIUS_REGEX.test(radius);🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code style issues found. Prettier formatting rules are not met.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/main.ts(2 hunks)lib/types.ts(1 hunks)
🧰 Additional context used
📓 Learnings (1)
lib/main.ts (2)
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:424-428
Timestamp: 2025-01-10T18:48:54.608Z
Learning: In the Kinde infrastructure codebase, console.log statements are intentionally used in error handling to provide detailed feedback to developers through the dashboard logs. These logs are only visible in the dashboard environment and serve as a debugging aid.
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:348-349
Timestamp: 2025-01-10T14:04:01.690Z
Learning: In the infrastructure repository, both sets of URL functions (getKindeSignUpUrl/getKindeSignInUrl and getKindeRegisterUrl/getKindeLoginUrl) should be maintained as active, non-deprecated APIs.
🪛 GitHub Actions: Build and test
lib/main.ts
[warning] Code style issues found. Prettier formatting rules are not met.
🔇 Additional comments (6)
lib/types.ts (3)
147-155: LGTM! Well-structured type definition.The
KindeDesignerCustomPropertiestype is well-defined with appropriate optional properties for design customization.
157-157: LGTM! Strong type safety with template literal.The
OrgCodetype effectively enforces theorg_prefix format using template literal types.
159-171: LGTM! Well-designed union type for mutual exclusivity.The type effectively ensures that users provide either direct client credentials or environment variable keys, but not both.
lib/main.ts (3)
341-401: LGTM! URL functions implementation aligns with requirements.Both sets of URL functions (register/login and signup/signin) are maintained as active APIs, as intended.
🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code style issues found. Prettier formatting rules are not met.
407-433: LGTM! Consistent implementation of asset URL functions.The asset URL functions are well-documented and follow a consistent pattern using the
getAssetUrlutility.🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code style issues found. Prettier formatting rules are not met.
460-509: LGTM! Well-implemented custom properties function.The function includes thorough validation, appropriate error handling with developer feedback (via console.log as intended), and clean CSS property generation.
🧰 Tools
🪛 GitHub Actions: Build and test
[warning] Code style issues found. Prettier formatting rules are not met.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/main.ts (3)
344-347: Enhance function documentationThe JSDoc comments for these functions lack meaningful descriptions of their purpose and usage context.
Example improvement for
getKindeWidget:/** - * - * @returns Kinde placeholder for the main page content + * Returns a placeholder that will be replaced with the main Kinde widget content + * during rendering. This widget contains the core UI components. + * + * @returns {KindePlaceholder} A placeholder string that will be replaced with the widget */Also applies to: 351-354, 358-361, 365-368, 372-375
403-433: Improve asset function documentationThe asset management functions would benefit from more detailed documentation explaining their purpose and usage.
Example improvement for
getLogoUrl:/** - * - * @returns Light Mode Logo Placeholder + * Generates a URL for retrieving the organization's light mode logo. + * + * @param {OrgCode} orgCode - The organization's unique identifier + * @returns {string} URL to the organization's light mode logo with cache busting */
459-508: Consider adding default values for optional propertiesThe function handles undefined values well, but consider providing default values for common properties to ensure consistent styling.
export const setKindeDesignerCustomProperties = ({ baseBackgroundColor, baseLinkColor, buttonBorderRadius = '4px', primaryButtonBackgroundColor, primaryButtonColor, cardBorderRadius = '8px', inputBorderRadius = '4px', }: KindeDesignerCustomProperties) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/main.ts(2 hunks)
🧰 Additional context used
📓 Learnings (1)
lib/main.ts (2)
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:424-428
Timestamp: 2025-01-10T18:48:54.608Z
Learning: In the Kinde infrastructure codebase, console.log statements are intentionally used in error handling to provide detailed feedback to developers through the dashboard logs. These logs are only visible in the dashboard environment and serve as a debugging aid.
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:348-349
Timestamp: 2025-01-10T14:04:01.690Z
Learning: In the infrastructure repository, both sets of URL functions (getKindeSignUpUrl/getKindeSignInUrl and getKindeRegisterUrl/getKindeLoginUrl) should be maintained as active, non-deprecated APIs.
🔇 Additional comments (1)
lib/main.ts (1)
435-444: LGTM! Well-implemented validation functionsThe color and border radius validation functions are well-structured and use appropriate regular expressions for validation.
DanielRivers
force-pushed
the
feat/custom-ui-updates
branch
from
99d9cbe to
8acc264
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/main.ts (2)
347-404: Enhance function documentation with meaningful descriptions.The JSDoc comments for the URL functions (
getKindeRegisterUrl,getKindeLoginUrl, etc.) currently have empty descriptions. Consider adding meaningful documentation explaining their purpose and usage./** - * - * @returns Register URL Placeholder + * Returns a placeholder for the registration URL. + * This function is used in templates to generate the final registration URL at runtime. + * @returns {KindePlaceholder} A placeholder string that will be replaced with the actual registration URL */
406-436: Enhance asset URL function documentation.The asset URL functions (
getLogoUrl,getDarkModeLogoUrl, etc.) would benefit from more detailed documentation explaining their purpose and usage./** - * - * @returns Light Mode Logo Placeholder + * Returns the URL for the organization's light mode logo. + * @param {OrgCode} orgCode - The organization's unique identifier + * @returns {string} The URL to the organization's light mode logo */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/main.ts(2 hunks)
🧰 Additional context used
📓 Learnings (1)
lib/main.ts (2)
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:424-428
Timestamp: 2025-01-10T18:48:54.608Z
Learning: In the Kinde infrastructure codebase, console.log statements are intentionally used in error handling to provide detailed feedback to developers through the dashboard logs. These logs are only visible in the dashboard environment and serve as a debugging aid.
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:348-349
Timestamp: 2025-01-10T14:04:01.690Z
Learning: In the infrastructure repository, both sets of URL functions (getKindeSignUpUrl/getKindeSignInUrl and getKindeRegisterUrl/getKindeLoginUrl) should be maintained as active, non-deprecated APIs.
🔇 Additional comments (2)
lib/main.ts (2)
7-14: LGTM! Import statements are well-organized.The new types are properly imported and used throughout the file.
20-25: LGTM! Secure implementation of asset URL construction.The function includes proper validation against path traversal attacks and handles organization codes correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lib/main.ts (4)
20-25: Enhance asset URL security and caching.While the basic path traversal protection is good, consider these improvements:
- The cache parameter uses a hardcoded value which might need to be configurable
- The path validation could be more robust
Consider this enhanced implementation:
const getAssetUrl = (assetPath: string, orgCode?: OrgCode) => { - if (!assetPath || assetPath.includes("..")) { + if (!assetPath || /[<>:"|?*]|\.\./.test(assetPath)) { throw new Error("Invalid asset path"); } - return `/${assetPath}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=@8973ff883c2c40e1bad198b543e12b24@`; + const cacheKey = process.env.ASSET_CACHE_KEY || "@8973ff883c2c40e1bad198b543e12b24@"; + return `/${assetPath.replace(/^\/+/, '')}?${orgCode ? `p_org_code=${orgCode}&` : ""}cache=${cacheKey}`; };
347-350: Improve JSDoc descriptions.The JSDoc comments for placeholder functions lack meaningful descriptions of their purpose.
Consider enhancing the documentation:
/** - * - * @returns Kinde placeholder for the main page content + * Returns a placeholder that will be replaced with the main Kinde widget content during rendering. + * @returns {KindePlaceholder} A placeholder string for the widget content */Apply similar improvements to other placeholder function docs.
Also applies to: 354-357, 361-364, 368-371, 375-378
410-412: Add error handling for undefined orgCode.The asset URL functions should validate the orgCode parameter.
Consider this enhancement:
export const getLogoUrl = (orgCode: OrgCode) => { + if (!orgCode?.trim()) { + throw new Error("Organization code is required"); + } return getAssetUrl("logo", orgCode); };Apply similar validation to other asset URL functions.
Also applies to: 418-420, 426-428, 434-436
438-447: Extract regex patterns as constants.The validation functions use inline regex patterns which could be moved to constants for better maintainability.
Consider this refactor:
+const COLOR_PATTERN = /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$|^rgb\(.*\)$|^rgba\(.*\)$/; +const BORDER_RADIUS_PATTERN = /^\d+(%|px|rem|em)$/; const isValidColor = (color: string | undefined) => - !color || /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$|^rgb\(.*\)$|^rgba\(.*\)$/.test(color); + !color || COLOR_PATTERN.test(color); const isValidBorderRadius = (radius: string | undefined) => - !radius || /^\d+(%|px|rem|em)$/.test(radius); + !radius || BORDER_RADIUS_PATTERN.test(radius);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/main.ts(2 hunks)
🧰 Additional context used
📓 Learnings (1)
lib/main.ts (2)
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:424-428
Timestamp: 2025-01-10T18:48:54.608Z
Learning: In the Kinde infrastructure codebase, console.log statements are intentionally used in error handling to provide detailed feedback to developers through the dashboard logs. These logs are only visible in the dashboard environment and serve as a debugging aid.
Learnt from: DanielRivers
PR: kinde-oss/infrastructure#28
File: lib/main.ts:348-349
Timestamp: 2025-01-10T14:04:01.690Z
Learning: In the infrastructure repository, both sets of URL functions (getKindeSignUpUrl/getKindeSignInUrl and getKindeRegisterUrl/getKindeLoginUrl) should be maintained as active, non-deprecated APIs.
🔇 Additional comments (2)
lib/main.ts (2)
7-14: LGTM! Clean import organization.The new imports are well-structured and properly typed.
449-511: LGTM! Well-implemented custom properties handling.The implementation includes:
- Proper validation of all input parameters
- Clear error messages with debugging information
- Clean CSS property generation
Explain your changes
Extra functions and tools for upcoming custom UI feature
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.