Add support for embedded Avatars with a Avatar spec#34
Conversation
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (60)
📒 Files selected for processing (27)
📝 WalkthroughWalkthroughAdds avatar URI parsing and SVG generation for gradient, anonymous animal, and anonymous entity avatars. Adds logo resolution across emoji, avatar, and URL schemes, integrates it into React and Vue components, exposes browser APIs, and replaces browser random byte generation with Web Crypto. ChangesAvatar and logo resolution
Browser randomness update
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Consumer
participant resolveLogoUri
participant AvatarParser
participant AvatarGenerator
Consumer->>resolveLogoUri: resolve logo specification
resolveLogoUri->>AvatarParser: parse avatar URI
AvatarParser-->>resolveLogoUri: return avatar parameters
resolveLogoUri->>AvatarGenerator: generate avatar data URI
AvatarGenerator-->>resolveLogoUri: return image source
resolveLogoUri-->>Consumer: return resolved logo
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/vue/src/components/primitives/Logo/Logo.ts (1)
45-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove invalid HTML attributes from the emoji
span.The
heightandwidthattributes are not valid on a<span>element. They will be passed to the DOM but will not natively affect the size of the emoji, as emoji dimensions are controlled by CSSfont-size. Consider removing them to keep the DOM output valid and prevent misleading behavior.🧹 Proposed cleanup
? h( 'span', { 'aria-label': props.alt, class: withVendorCSSClassPrefix('logo__emoji'), - height: props.height, role: 'img', - width: props.width, }, [resolvedLogo.glyph], )🤖 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/vue/src/components/primitives/Logo/Logo.ts` around lines 45 - 55, Remove the height and width attributes from the emoji span created in the Logo component’s resolvedLogo glyph branch. Keep the aria-label, class, role, and glyph content unchanged; emoji sizing should remain controlled by the existing CSS.
🤖 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/browser/src/utils/generateAvatarDataUri.ts`:
- Around line 47-53: Extract the duplicated hashStr implementation into one
shared utility and import it wherever needed. Update
packages/browser/src/utils/generateAvatarDataUri.ts lines 47-53 to use the
shared export, remove the duplicate from
packages/browser/src/utils/pickAnonymousAvatarName.ts lines 23-29 and import the
utility, and do the same in
packages/browser/src/utils/pickAnonymousEntityName.ts lines 23-29; preserve the
existing hashStr behavior and ensure the shared module re-exports it as needed.
- Around line 137-138: Replace the direct truthiness lookup in
generateAnonymousAnimalAvatar with
Object.prototype.hasOwnProperty.call(ANONYMOUS_ANIMAL_ICONS, content) before
selecting the key. Apply the same membership check in
generateAnonymousEntityAvatar for ANONYMOUS_ENTITY_ICONS, covering both
referenced sites in packages/browser/src/utils/generateAvatarDataUri.ts;
preserve the existing fallback behavior for unknown keys.
In `@packages/browser/src/utils/SPACryptoUtils.ts`:
- Around line 66-68: Update generateRandomBytes() to fill the requested buffer
through multiple crypto.getRandomValues() calls, limiting each temporary
Uint8Array request to the API’s 65,536-byte maximum. Preserve the existing
Buffer return type and support requests larger than that limit without throwing.
In `@packages/react/src/components/primitives/Avatar/Avatar.tsx`:
- Around line 141-144: Update the emoji branch in the Avatar rendering logic
around resolvedIcon to return a span with role="img", preserving the emoji glyph
as its content and applying the avatar’s alt text for accessibility instead of
returning a bare string.
In `@packages/vue/src/components/primitives/Logo/Logo.css.ts`:
- Around line 50-57: Replace the hardcoded `.thunderid-logo__emoji` selector
with a prefix-independent or dynamically vendor-prefixed selector that matches
the class generated by `withVendorCSSClassPrefix('logo__emoji')`; use the
existing vendor configuration or `getVendorPrefix()` utility if runtime scoping
is required.
---
Nitpick comments:
In `@packages/vue/src/components/primitives/Logo/Logo.ts`:
- Around line 45-55: Remove the height and width attributes from the emoji span
created in the Logo component’s resolvedLogo glyph branch. Keep the aria-label,
class, role, and glyph content unchanged; emoji sizing should remain controlled
by the existing CSS.
🪄 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: c4e3e1b3-2729-44e6-bc8d-2c9373a2a07c
⛔ Files ignored due to path filters (60)
packages/browser/src/utils/anonymousAnimalIcons.generated.tsis excluded by!**/*.generated.*packages/browser/src/utils/anonymousEntityIcons.generated.tsis excluded by!**/*.generated.*packages/browser/static/images/avatars/anonymous_animals/capybara.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/chameleon.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/chinchilla.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/chipmunk.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/chupacabra.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/dingo.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/dinosaur.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/frog.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/giraffe.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/hedgehog.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/hippo.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/jackal.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/jackalope.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/mink.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/otter.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/platypus.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/quagga.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/raccoon.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_animals/skunk.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/anchor.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/antenna.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/anvil.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/arch.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/bridge.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/chevron.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/circuit_node.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/compass.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/cube.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/diamond.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/dome.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/gate.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/hexagon.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/key.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/lighthouse.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/lock.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/obelisk.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/octagon.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/orbit_ring.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/parallelogram.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/pavilion.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/pentagon.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/plus_facet.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/silo.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/spiral.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/spire.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/star.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/tower.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/townhouse.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/triangle_stack.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/turbine.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/valve.svgis excluded by!**/*.svgpackages/browser/static/images/avatars/anonymous_entity/windmill.svgis excluded by!**/*.svgpackages/react/.vitest-attachments/27677941df85b03d5d2cfa78a06ab03580dd9ac6.pngis excluded by!**/*.pngpackages/react/.vitest-attachments/3dab8c5a94eed1b7587e7dbb5fed1db2c644801d.pngis excluded by!**/*.pngpackages/react/.vitest-attachments/5a7d509e64e503baebcb9f280cb5bd323b6d722e.pngis excluded by!**/*.pngpackages/react/src/components/adapters/__tests__/__screenshots__/ImageComponent.test.tsx/ImageComponent-renders-an-emoji--spec-as-its-glyph--not-an-image-1.pngis excluded by!**/*.pngpackages/react/src/components/auth/Callback/__tests__/__screenshots__/TokenCallback.test.tsx/TokenCallback-verifies-the-token-as-a-registration-flow-when-type-REGISTRATION-is-present-1.pngis excluded by!**/*.pngpackages/react/src/components/auth/Callback/__tests__/__screenshots__/TokenCallback.test.tsx/TokenCallback-verifies-the-token-as-an-authentication-flow-when-type-AUTHENTICATION-is-present-1.pngis excluded by!**/*.png
📒 Files selected for processing (26)
packages/browser/scripts/generate-anonymous-animal-icons.mjspackages/browser/scripts/generate-anonymous-entity-icons.mjspackages/browser/src/index.tspackages/browser/src/utils/SPACryptoUtils.tspackages/browser/src/utils/__tests__/extractAvatarParamsFromUri.test.tspackages/browser/src/utils/__tests__/generateAvatarDataUri.test.tspackages/browser/src/utils/__tests__/logoUriSchemes.test.tspackages/browser/src/utils/__tests__/pickAnonymousAvatarName.test.tspackages/browser/src/utils/__tests__/pickAnonymousEntityName.test.tspackages/browser/src/utils/__tests__/resolveLogoUri.test.tspackages/browser/src/utils/extractAvatarParamsFromUri.tspackages/browser/src/utils/generateAvatarDataUri.tspackages/browser/src/utils/isAvatarUri.tspackages/browser/src/utils/pickAnonymousAvatarName.tspackages/browser/src/utils/pickAnonymousEntityName.tspackages/browser/src/utils/resolveLogoUri.tspackages/react/src/components/adapters/ImageComponent.tsxpackages/react/src/components/adapters/__tests__/ImageComponent.test.tsxpackages/react/src/components/presentation/UserAvatar/BaseUserAvatar.tsxpackages/react/src/components/primitives/Avatar/Avatar.tsxpackages/react/src/components/primitives/Avatar/__tests__/Avatar.test.tsxpackages/react/src/components/primitives/Logo/Logo.styles.tspackages/react/src/components/primitives/Logo/Logo.tsxpackages/react/src/index.tspackages/vue/src/components/primitives/Logo/Logo.css.tspackages/vue/src/components/primitives/Logo/Logo.ts
|
|
||
| .thunderid-logo__emoji { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| line-height: 1; | ||
| font-size: 1.75em; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Avoid hardcoded vendor prefixes in CSS class names.
This CSS block hardcodes the .thunderid- prefix (.thunderid-logo__emoji). Because the Logo component dynamically computes the class name via withVendorCSSClassPrefix('logo__emoji'), this static style rule will fail to match if a different vendor prefix is configured at runtime.
As per path instructions, ask: "Should this read from the vendor config / getVendorPrefix() util instead of hardcoding the vendor name?" Consider avoiding the vendor prefix entirely if it isn't load-bearing, or find an alternative way to ensure the styles are scoped dynamically (e.g., dynamically interpolating getVendorPrefix(vendor) into the string).
🤖 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/vue/src/components/primitives/Logo/Logo.css.ts` around lines 50 -
57, Replace the hardcoded `.thunderid-logo__emoji` selector with a
prefix-independent or dynamically vendor-prefixed selector that matches the
class generated by `withVendorCSSClassPrefix('logo__emoji')`; use the existing
vendor configuration or `getVendorPrefix()` utility if runtime scoping is
required.
Source: Path instructions
There was a problem hiding this comment.
Need to fix this separately. @coderabbitai Create an issue to track
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="thunder-id/javascript-sdks"
title="Fix vendor-prefixed emoji selector in Vue Logo styles"
body=$(cat <<'EOF'
## Summary
The Vue Logo emoji style selector is statically scoped as `.thunderid-logo__emoji`, while the Logo component generates the corresponding class through `withVendorCSSClassPrefix('logo__emoji')`.
## Required changes
Update the emoji styling approach so it matches the runtime-configured vendor CSS class prefix. Avoid a hardcoded vendor prefix when it is not load-bearing; otherwise derive the prefix through the existing vendor configuration/utilities.
## Rationale
Consumers using a vendor prefix other than `thunderid` will render the emoji element with a class that does not match the static CSS selector, so its layout and typography styles will not apply.
## Affected area
- `packages/vue/src/components/primitives/Logo/Logo.css.ts`
- Vue Logo class generation in `packages/vue/src/components/primitives/Logo/Logo.ts`
## Acceptance criteria
- Emoji logo styles apply when a custom vendor prefix is configured.
- The implementation follows the repository vendor-prefix guidance and does not hardcode `thunderid` for a consumer-configurable runtime class name.
- Existing default-vendor Logo styling remains intact.
## Backlinks
- Pull request: https://github.com/thunder-id/javascript-sdks/pull/34
- Review comment: https://github.com/thunder-id/javascript-sdks/pull/34#discussion_r3617377888
Requested by: `@brionmario`
EOF
)
gh issue create \
--repo "$repo" \
--title "$title" \
--body "$body" \
--assignee "brionmario"Length of output: 1966
@brionmario, I couldn’t create the follow-up: GitHub Issues are disabled for thunder-id/javascript-sdks.
Please enable Issues for the repository or point me to the preferred external tracker, and I can create it there.
|
Addressed CodeRabbit's actionable comments in dc4394b:
Skipped: the hardcoded All affected unit tests pass and |
Purpose
Adds support for generating and rendering embedded avatars (initials, blank, and curated anonymous animal/entity icons) as a first-class
avatar:logo spec, alongside the existingemoji:scheme, and wires it through React'sLogo/Avatar/BaseUserAvatarand Vue'sLogocomponents so both platforms render every logo spec type consistently.Approach
avatar:URI scheme in@thunderid/browser(isAvatarUri,extractAvatarParamsFromUri,generateAvatarDataUri) parsed as comma-separatedkey=valuepairs:shape,variant,content,colors,bg.one_letter/two_letter/blank(gradient or flat-color swatches) andanonymous_animal/anonymous_entity(curated SVG icon sets, generated at build time from source SVGs intoanonymousAnimalIcons.generated.ts/anonymousEntityIcons.generated.tsviascripts/generate-anonymous-*-icons.mjs).pickAnonymousAvatarName/pickAnonymousEntityName, which deterministically hash a seed (e.g. a session ID) to the same curated icon every time, so anonymous users/sessions keep a stable avatar across renders.resolveLogoUrias the single entry point that triesemoji:→avatar:→ falls back to treating the spec as a plain URL, so callers render "whatever a logo spec is" without special-casing each scheme.resolveLogoUriinto React'sLogo,Avatar, and the embedded-flowImageComponentadapter, and Vue'sLogo, replacing the previous emoji-only special-casing.BaseUserAvatarnow shows a deterministic anonymous avatar (viapickAnonymousAvatarName) instead of a static default whenuserisnull.SPACryptoUtils.generateRandomBytesto use the native Web Cryptocrypto.getRandomValuesinstead of therandombytesnpm package, dropping an extra dependency and its non-CSPRNG fallback behavior in unsupported environments.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks