feat(content-insights): add isRedesignEnabled flag for Blueprint button #4420
Conversation
|
|
WalkthroughThis change introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
Can we add some VRT tests here? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/content-insights/OpenContentInsightsButton.tsx (1)
14-35: Correct the BlueprintButtonsizeprop value.The implementation correctly follows React hooks rules by calling
useIntlunconditionally at the component top. The early return pattern provides clean separation between the two rendering paths while maintaining consistentclassNamefor styling and test selectors.However, the
size="small"prop is incorrect. According to the@box/blueprint-webButton API, valid size values are'sm','default', or'lg'. Changesize="small"tosize="sm"on line 20. Thevariant="secondary"prop is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/features/content-insights/CompactCount.tsx`:
- Line 24: CompactCount.tsx passes a redundant string into the classNames call
causing a leading space; remove the unnecessary `' '` argument so the JSX uses
classNames(className) (locate the className={classNames(' ', className)} usage
in the CompactCount component and replace it with a single-argument call to
classNames or equivalent handling of className).
🧹 Nitpick comments (4)
src/features/content-insights/messages.ts (1)
78-82: Inconsistent casing with other message defaults.
previewGraphTypenow uses title case ('Previews') while similar messages likedownloadGraphType('DOWNLOADS') andpeopleTitle('PEOPLE') remain in all caps. If the redesign requires title case, consider updating all related messages for consistency, or ensure the casing is handled via CSStext-transformto avoid divergence.src/features/content-insights/CompactCount.tsx (1)
34-37: ReuseformattedCountin the non-redesigned branch.
formattedCountis already computed at line 18 but the non-redesigned path callsformatCount(count, intl)again. Reuse the variable for consistency and to avoid redundant computation.Proposed fix
return ( <span className={classNames('CompactCount', className)} {...rest}> - {formatCount(count, intl)} + {formattedCount} </span> );src/features/content-insights/stories/tests/ContentInsightsSummary-visual.stories.tsx (1)
65-74: Consider adding aWithErrorRedesignstory.The error state is only tested without the redesign flag. For comprehensive VRT coverage, consider adding a story that tests the error state with
isRedesignEnabled={true}to ensure the error UI renders correctly in the redesigned layout.Suggested addition
export const WithErrorRedesign = { render: () => ( <Wrapper> <ContentInsightsSummary {...defaultProps} error={Object.assign(new Error('An error occurred'), { status: 500 })} isRedesignEnabled /> </Wrapper> ), };src/features/content-insights/MetricSummary.tsx (1)
3-4: Pre-existing: Shadowing of globalisFiniteandisNaN.Static analysis flags these imports as shadowing global names. While this is intentional (lodash versions handle edge cases like
isNaN(undefined)differently), consider aliasing them to avoid confusion:import isFiniteLodash from 'lodash/isFinite'; import isNaNLodash from 'lodash/isNaN';This is pre-existing code and not blocking for this PR.
Merge Queue Status✅ The pull request has been merged at e1e381c This pull request spent 7 seconds in the queue, with no time running CI. Required conditions to merge
|
5 participants
Passes the
isRedesignEnabledfeature flag through the Content Insights component tree so child components can conditionally render Blueprint vs BUIE variants. This keeps the toggle controlled by the end‑user app and enables gradual migration to Blueprint across the Content Insights UI.Changes
isRedesignEnabledprop toContentInsightsSummary→GraphCardPreviewsSummary→MetricSummary→HeaderWithCount/CompactCount/OpenContentInsightsButtonButtonandTextvariants when redesign is enabledContentInsightsSummarySuggestionForNewlyCreatedTemplateInstance) withwaitForUsage
Screenshot
Summary by CodeRabbit
Release Notes
New Features
Style
Other
✏️ Tip: You can customize this high-level summary in your review settings.