Add "ThunderID" attribution badge support - #44
Conversation
📝 WalkthroughWalkthroughThe PR adds configurable “Powered by ThunderID” attribution to React authentication surfaces, shared browser branding and theme tokens, localized attribution text, provider/component visibility precedence, and centralized elevated-surface styling for several presentation components. ChangesAttribution and elevated surface design system
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AuthSurface
participant useAttribution
participant ThunderIDProvider
participant Attribution
participant Theme
participant Translation
AuthSurface->>useAttribution: pass component showAttribution
useAttribution->>ThunderIDProvider: read provider setting
useAttribution-->>AuthSurface: return resolved visibility
AuthSurface->>Attribution: render when enabled
Attribution->>Theme: read border and shadow values
Attribution->>Translation: resolve powered_by label
Translation-->>Attribution: return localized label
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/react/src/index.ts (1)
230-231: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute the package export through the new Attribution entrypoint.
Use
./components/primitives/Attributioninstead of importing the implementation file directly, so future exports from the primitive’sindex.tsare not bypassed.Suggested change
-export {default as Attribution} from './components/primitives/Attribution/Attribution'; -export * from './components/primitives/Attribution/Attribution'; +export {default as Attribution} from './components/primitives/Attribution'; +export * from './components/primitives/Attribution';As per coding guidelines, check each package's
src/index.tsre-exports before adding or duplicating a helper; dependency exports should be reused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react/src/index.ts` around lines 230 - 231, Update the Attribution exports in the package entrypoint to import from the directory entrypoint ./components/primitives/Attribution rather than the implementation file, preserving both the default and wildcard re-exports through that index.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react/src/components/presentation/auth/SignIn/BaseSignIn.styles.ts`:
- Around line 39-45: Update the card styles in
packages/react/src/components/presentation/auth/SignIn/BaseSignIn.styles.ts
(lines 39-45) and
packages/react/src/components/presentation/auth/SignUp/BaseSignUp.styles.ts
(lines 39-45) to include the shared surface.border token alongside the existing
elevated surface properties.
In `@packages/react/src/components/primitives/Attribution/Attribution.tsx`:
- Line 65: Update the data-testid on the attribution container in the
Attribution component to use the neutral identifier "attribution" instead of the
hardcoded vendor-specific name.
- Around line 57-65: Update the Attribution component’s theme-dependent styling
around buildAttributionCss and ATTRIBUTION_BLOCK so each badge instance scopes
its border and shadow values independently. Use per-instance CSS variables or a
generated unique class, and ensure the injected selectors target only that
instance rather than overwriting styles on other badges.
---
Nitpick comments:
In `@packages/react/src/index.ts`:
- Around line 230-231: Update the Attribution exports in the package entrypoint
to import from the directory entrypoint ./components/primitives/Attribution
rather than the implementation file, preserving both the default and wildcard
re-exports through that index.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 835ba8a9-d356-490b-8693-d2a005107d3d
📒 Files selected for processing (28)
packages/browser/src/branding/attributionDesign.tspackages/browser/src/branding/resolveShowAttribution.tspackages/browser/src/index.tspackages/browser/src/theme/elevatedSurface.tspackages/javascript/src/i18n/models/i18n.tspackages/javascript/src/i18n/translations/en-US.tspackages/javascript/src/i18n/translations/fr-FR.tspackages/javascript/src/i18n/translations/hi-IN.tspackages/javascript/src/i18n/translations/ja-JP.tspackages/javascript/src/i18n/translations/pt-BR.tspackages/javascript/src/i18n/translations/pt-PT.tspackages/javascript/src/i18n/translations/si-LK.tspackages/javascript/src/i18n/translations/ta-IN.tspackages/javascript/src/i18n/translations/te-IN.tspackages/javascript/src/models/config.tspackages/react/src/components/presentation/UserDropdown/BaseUserDropdown.styles.tspackages/react/src/components/presentation/UserProfile/BaseUserProfile.styles.tspackages/react/src/components/presentation/auth/Recovery/BaseRecovery.tsxpackages/react/src/components/presentation/auth/SignIn/BaseSignIn.styles.tspackages/react/src/components/presentation/auth/SignIn/BaseSignIn.tsxpackages/react/src/components/presentation/auth/SignUp/BaseSignUp.styles.tspackages/react/src/components/presentation/auth/SignUp/BaseSignUp.tsxpackages/react/src/components/primitives/Attribution/Attribution.tsxpackages/react/src/components/primitives/Attribution/index.tspackages/react/src/contexts/ThunderID/ThunderIDContext.tspackages/react/src/contexts/ThunderID/ThunderIDProvider.tsxpackages/react/src/hooks/useAttribution.tspackages/react/src/index.ts
| const card: string = css` | ||
| background: ${theme.vars.colors.background.surface}; | ||
| border-radius: ${theme.vars.borderRadius.large}; | ||
| background: ${surface.background}; | ||
| border-radius: ${surface.borderRadius}; | ||
| box-shadow: ${surface.boxShadow}; | ||
| gap: calc(${theme.vars.spacing.unit} * 2); | ||
| min-width: 420px; | ||
| padding: ${surface.padding}; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'BaseSignIn\.styles\.ts|BaseSignUp\.styles\.ts' . || true
echo "== relevant file contents =="
for f in $(fd 'BaseSignIn\.styles\.ts|BaseSignUp\.styles\.ts' .); do
echo "--- $f ---"
wc -l "$f"
sed -n '1,120p' "$f" | cat -n
done
echo "== surface.border definitions/usages =="
rg -n "border: \$\{surface\.border\}|surface\.border|background: \$\{surface\.background\}|component.*surface|const surface|interface .*Surface|surface =" -S .Repository: thunder-id/javascript-sdks
Length of output: 11341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ElevatedSurfaceTokens definition =="
sed -n '1,120p' packages/browser/src/theme/elevatedSurface.ts | cat -n
echo "== getElevatedSurfaceTokens usages around border =="
while IFS= read -r f; do
echo "--- $f ---"
wc -l "$f"
rg -n -C 8 "getElevatedSurfaceTokens|border: \$\{surface\.border\}|padding: \$\{surface\.padding" "$f" || true
done < <(git ls-files 'packages/react/src/components/presentation/**/*.ts' | grep -E 'BaseUser(P|D).*\.styles\.ts|SignIn/.*\.styles\.ts|SignUp/.*\.styles\.ts')Repository: thunder-id/javascript-sdks
Length of output: 3155
Include surface.border on both authentication cards.
getElevatedSurfaceTokens() returns a shared 1px border token for top-level card surfaces, and this is already used by the profile/dropdown elevated surfaces. Add the same border to both BaseSignIn.styles.ts and BaseSignUp.styles.ts card rules.
🧰 Tools
🪛 ESLint
[error] 40-40: Unsafe argument of type error typed assigned to a parameter of type CSSInterpolation.
(@typescript-eslint/no-unsafe-argument)
[error] 40-40: Unsafe member access .background on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
[error] 41-41: Unsafe argument of type error typed assigned to a parameter of type CSSInterpolation.
(@typescript-eslint/no-unsafe-argument)
[error] 41-41: Unsafe member access .borderRadius on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
[error] 42-42: Unsafe argument of type error typed assigned to a parameter of type CSSInterpolation.
(@typescript-eslint/no-unsafe-argument)
[error] 42-42: Unsafe member access .boxShadow on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
[error] 45-45: Unsafe argument of type error typed assigned to a parameter of type CSSInterpolation.
(@typescript-eslint/no-unsafe-argument)
[error] 45-45: Unsafe member access .padding on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
📍 Affects 2 files
packages/react/src/components/presentation/auth/SignIn/BaseSignIn.styles.ts#L39-L45(this comment)packages/react/src/components/presentation/auth/SignUp/BaseSignUp.styles.ts#L39-L45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react/src/components/presentation/auth/SignIn/BaseSignIn.styles.ts`
around lines 39 - 45, Update the card styles in
packages/react/src/components/presentation/auth/SignIn/BaseSignIn.styles.ts
(lines 39-45) and
packages/react/src/components/presentation/auth/SignUp/BaseSignUp.styles.ts
(lines 39-45) to include the shared surface.border token alongside the existing
elevated surface properties.
| const css: string = useMemo( | ||
| () => buildAttributionCss(theme.vars.colors.border, theme.vars.shadows.medium), | ||
| [theme.vars.colors.border, theme.vars.shadows.medium], | ||
| ); | ||
|
|
||
| return ( | ||
| <> | ||
| <style>{css}</style> | ||
| <div className={cx(ATTRIBUTION_BLOCK, className)} data-testid="thunderid-attribution"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the theme-dependent badge styles per instance.
Every badge injects the same global selectors, so a later badge with different component theme preferences overwrites the border and shadow of earlier badges. Use per-instance CSS variables or a generated scoped class.
🧰 Tools
🪛 ESLint
[error] 57-60: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 58-58: Unsafe return of a value of type error.
(@typescript-eslint/no-unsafe-return)
[error] 58-58: Unsafe call of a type that could not be resolved.
(@typescript-eslint/no-unsafe-call)
[error] 65-65: Unsafe argument of type error typed assigned to a parameter of type ClassNamesArg.
(@typescript-eslint/no-unsafe-argument)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react/src/components/primitives/Attribution/Attribution.tsx` around
lines 57 - 65, Update the Attribution component’s theme-dependent styling around
buildAttributionCss and ATTRIBUTION_BLOCK so each badge instance scopes its
border and shadow values independently. Use per-instance CSS variables or a
generated unique class, and ensure the injected selectors target only that
instance rather than overwriting styles on other badges.
| return ( | ||
| <> | ||
| <style>{css}</style> | ||
| <div className={cx(ATTRIBUTION_BLOCK, className)} data-testid="thunderid-attribution"> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the hardcoded vendor name from the test ID.
Use a neutral identifier such as data-testid="attribution"; this DOM runtime name is not load-bearing for branding.
🧰 Tools
🪛 ESLint
[error] 65-65: Unsafe argument of type error typed assigned to a parameter of type ClassNamesArg.
(@typescript-eslint/no-unsafe-argument)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react/src/components/primitives/Attribution/Attribution.tsx` at line
65, Update the data-testid on the attribution container in the Attribution
component to use the neutral identifier "attribution" instead of the hardcoded
vendor-specific name.
Sources: Coding guidelines, Path instructions
Purpose
Approach
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit