feat(classification): add Blueprint rendering behind isRedesignEnabled flag#4448
Merged
mergify[bot] merged 7 commits intobox:masterfrom Feb 19, 2026
Merged
Conversation
…dByAiDetails class in redesign
… and Classification
Contributor
WalkthroughThis PR introduces an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/features/classification/Classification.stories.tsx (1)
24-79: LGTM — good story coverage across permutationsThe matrix of Legacy/Redesigned × plain / with-controls / with-AI-classification gives solid visual regression coverage for the new flag. One gap to consider: an unclassified redesign story (no
nameprop) to exerciseisNotClassifiedMessageVisibleunderisRedesignEnabled, since it still uses the classic<span className="bdl-Classification-missingMessage">path regardless of the flag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/classification/Classification.stories.tsx` around lines 24 - 79, Add a new story that renders the redesigned variant of <Classification> with isRedesignEnabled set but without providing any name/itemName prop so the unclassified state is exercised; mirror the existing Redesigned story (use isRedesignEnabled) but omit itemName/name and any aiClassificationReason/controls so the component's isNotClassifiedMessageVisible path (and the <span className="bdl-Classification-missingMessage"> output) is rendered for visual tests.src/features/classification/security-controls/SecurityControls.js (1)
119-134:SecurityControlsborrows CSS class names fromClassification's scope
bdl-Classification-propertySectionandbdl-Classification-sectionLabel(Lines 119, 122) are defined inClassification.scss, not inSecurityControls.scss. This creates an implicit cross-component coupling: renaming or moving those classes inClassification.scsswill silently break theSecurityControlsredesign layout without any compile-time error.Consider either:
- Defining equivalent classes in
SecurityControls.scss(e.g.bdl-SecurityControls-section,bdl-SecurityControls-sectionLabel), or- Passing a
renderLabelrender-prop fromClassificationso that the wrapping/labelling is entirely owned by the Classification component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/classification/security-controls/SecurityControls.js` around lines 119 - 134, SecurityControls is using Classification-scoped CSS classes (bdl-Classification-propertySection, bdl-Classification-sectionLabel) which couples the components; change SecurityControls to own its styles by renaming those classNames to component-scoped names (e.g., bdl-SecurityControls-section and bdl-SecurityControls-sectionLabel) in the JSX where itemsList and shouldShowSecurityControlsModal are rendered (within the SecurityControls component) and add corresponding rules in SecurityControls.scss to mirror the visual behavior; alternatively, if you prefer composition, add a renderLabel prop to Classification and update SecurityControls to accept and use that render prop instead of hardcoding label markup—pick one approach and make matching changes to the class names/SCSS or implement the renderLabel prop and update usages accordingly (ensure openModal and itemsList behavior remains unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/classification/Classification.js`:
- Around line 138-145: The redesign path replaces the semantic 'article' with a
non-semantic 'div' via the Wrapper variable, which removes a navigable landmark;
change the Wrapper assignment so that when isRedesignEnabled is true it uses
'section' (preserving landmark semantics) instead of 'div' (i.e., update the
const Wrapper = isRedesignEnabled ? 'div' : 'article' to use 'section' for the
redesign case) while leaving the className/classNames usage and JSX wrapper
unchanged.
- Around line 146-156: The redesign branch is dropping the onClick handler so
consumers lose the edit interaction; when isRedesignEnabled is true, either
forward the onClick to Status (e.g., wrap Status in a clickable wrapper or pass
through a prop that attaches the handler and
data-resin-target="editclassification") or add an explicit guard/comment and a
development-time console.warn to indicate edit is not supported; update the JSX
around isRedesignEnabled/Status (and keep ClassifiedBadge behavior unchanged) so
onClick is preserved or the omission is clearly signaled.
- Line 148: The Status component is being passed an arbitrary hex string via the
color prop (variable color) which violates the Status Color type; update the
code that sets/provides color to use one of the allowed design-token values
(e.g., SurfaceStatusSurfaceGray, SurfaceStatusSurfaceYellow,
SurfaceStatusSurfaceOrange, SurfaceStatusSurfaceRed, SurfaceStatusSurfacePurple,
SurfaceStatusSurfaceLightBlue, SurfaceStatusSurfaceDarkBlue,
SurfaceStatusSurfaceGreen) or replace Status with a component that accepts
arbitrary colors; locate where Status is rendered (Status color={color}
icon={Shield} iconPosition="left" text={name}) and either map the current hex
values to the correct token constants or change the source of color to return a
token name instead.
In `@src/features/classification/security-controls/SecurityControls.js`:
- Around line 117-150: The "View All" TextButton is currently rendered only
inside the shouldRenderLabel branch causing no trigger when
isRedesignEnabled=true and shouldRenderLabel=false; update the SecurityControls
component by removing the TextButton from inside the
shouldRenderLabel/isRedesignEnabled conditional (the block that sets itemsList)
and instead render a single conditional trigger in the return JSX alongside the
existing PlainButton: when isRedesignEnabled is true and
shouldShowSecurityControlsModal is true render the TextButton
(onClick={this.openModal}), and when isRedesignEnabled is false render the
existing PlainButton (onClick={this.openModal}); keep itemsList and
securityControlsModal unchanged and ensure openModal is used as the click
handler.
---
Nitpick comments:
In `@src/features/classification/Classification.stories.tsx`:
- Around line 24-79: Add a new story that renders the redesigned variant of
<Classification> with isRedesignEnabled set but without providing any
name/itemName prop so the unclassified state is exercised; mirror the existing
Redesigned story (use isRedesignEnabled) but omit itemName/name and any
aiClassificationReason/controls so the component's isNotClassifiedMessageVisible
path (and the <span className="bdl-Classification-missingMessage"> output) is
rendered for visual tests.
In `@src/features/classification/security-controls/SecurityControls.js`:
- Around line 119-134: SecurityControls is using Classification-scoped CSS
classes (bdl-Classification-propertySection, bdl-Classification-sectionLabel)
which couples the components; change SecurityControls to own its styles by
renaming those classNames to component-scoped names (e.g.,
bdl-SecurityControls-section and bdl-SecurityControls-sectionLabel) in the JSX
where itemsList and shouldShowSecurityControlsModal are rendered (within the
SecurityControls component) and add corresponding rules in SecurityControls.scss
to mirror the visual behavior; alternatively, if you prefer composition, add a
renderLabel prop to Classification and update SecurityControls to accept and use
that render prop instead of hardcoding label markup—pick one approach and make
matching changes to the class names/SCSS or implement the renderLabel prop and
update usages accordingly (ensure openModal and itemsList behavior remains
unchanged).
jpan-box
reviewed
Feb 19, 2026
jpan-box
reviewed
Feb 19, 2026
jpan-box
reviewed
Feb 19, 2026
jpan-box
approved these changes
Feb 19, 2026
Contributor
Merge Queue StatusRule:
This pull request spent 7 seconds in the queue, with no time running CI. Required conditions to merge
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Passes the
isRedesignEnabledfeature flag through theClassificationcomponent tree so child components can conditionally render Blueprint vs BUIE variants. This keeps the toggle controlled by the client app.Changes
isRedesignEnabledprop toClassification→SecurityControls→SecurityControlsItemUsage
Screenshots
Summary by CodeRabbit
Release Notes
New Features
Style