Conversation
…tles Insights titles and counts use captionBold when the host passes isRedesignEnabled. CompactCount accepts variant and color so the header chip can use bodyLargeBold. Details panel and section titles always use titleLarge and bodyDefaultBold.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. WalkthroughThe sidebar now forwards an optional redesign flag to content insights. Sidebar titles use Blueprint Web ChangesContent sidebar redesign
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant DetailsSidebar
participant SidebarContentInsights
participant ContentInsightsSummary
DetailsSidebar->>SidebarContentInsights: pass isRedesignEnabled
SidebarContentInsights->>ContentInsightsSummary: pass isRedesignEnabled
ContentInsightsSummary-->>SidebarContentInsights: render redesigned summary class
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The typography and redesign-flag changes have no established production, security, or data-integrity failure and are mergeable with normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 13 files. (1 skipped: 1 unsupported.)
✨ 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. A rabbit checks the sidebar bright Comment |
Panel headings use the 18px Inter token already shown in preview.
| if (isRedesignEnabled) { | ||
| return ( | ||
| <Text as="span" className={className} color="textOnLightSecondary" variant="bodyDefaultSemibold" {...rest}> | ||
| <Text as="span" className={className} color={color} variant={variant} {...rest}> |
There was a problem hiding this comment.
why are color and variant coming from props now?
instead of:
| <Text as="span" className={className} color={color} variant={variant} {...rest}> | |
| <Text as="span" className={className} color="textOnLightSecondary" variant="captionBold" {...rest}> |
| <Text as="span" variant="bodyDefaultBold"> | ||
| {title} | ||
| </Text> |
There was a problem hiding this comment.
out of curiosity, any visual difference here?
| {title && ( | ||
| <Text as="h2" className="bcs-title" variant="titleMedium"> | ||
| {title} | ||
| </Text> | ||
| )} |
There was a problem hiding this comment.
same question, any visual difference here? the native h2 would've had margin / padding on it but the Text from blueprint would reset those properties
There was a problem hiding this comment.
nvm I see in src/elements/content-sidebar/SidebarContent.scss that bcs-title removes the margin and padding. now that this is using Text, you don't need those properties anymore?
| jest.mock('@box/blueprint-web', () => ({ | ||
| Text: ({ | ||
| as: Component = 'p', | ||
| children, | ||
| variant, | ||
| ...rest | ||
| }: React.ComponentPropsWithoutRef<'span'> & { as?: React.ElementType; variant?: string }) => ( | ||
| <Component data-variant={variant} {...rest}> | ||
| {children} | ||
| </Component> | ||
| ), | ||
| })); |
There was a problem hiding this comment.
is the blueprint-web mock needed? I thought RTL tests would be able to use blueprint-web components without modification
2 participants
Summary
TextcaptionBoldwhen the host passesisRedesignEnabledvariantandcoloronCompactCountso the header chip can usebodyLargeBoldandtextOnLightDefaultTexttitleLargeand section headers onbodyDefaultBoldisRedesignEnabledthroughDetailsSidebarandSidebarContentInsightsinstead of hardcoding itSidebarContentandSidebarSectiontests to RTL TypeScriptWhy
Insights and Details sidebar type was still on raw spans and old CSS sizes. Blueprint
Textvariants match the Inter audit sizes. Insights stay host-gated; panel and section titles do not.Test plan
captionBoldwhen the host passesisRedesignEnableddetailsSidebarProps.isRedesignEnabledis trueh2usestitleLargebodyDefaultBoldvariant="bodyLargeBold"andcolor="textOnLightDefault"after this lands.bcs-titlewithout a variant. It may render atbodyDefault(14px) instead of the old 16px until that title sets a variant. Left that component unchanged.Summary by CodeRabbit
New Features
Style
Tests