fix(docgen-sidebar): adjust fixed accordion items#4500
fix(docgen-sidebar): adjust fixed accordion items#4500pyaromchyk-stack wants to merge 1 commit intobox:masterfrom
Conversation
WalkthroughThis PR updates DocGenSidebar styling and TagTree component to support the preview modernization feature. It replaces global accordion width/padding rules with CSS custom properties, introduces a modernized font-size variant for tag paths, and conditionally applies a modernized CSS class based on the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.scss (1)
52-55: Prefer setting accordion custom properties on the container, not*.Applying these vars on every descendant is overly broad. Set them on
.bcs-DocGen-collapsibleso they still inherit where needed with lower selector cost and less override risk.Suggested refactor
.bcs-DocGen-collapsible { min-width: 0; border: none; width: 100%; - - * { - --accordion-fixed-content-padding-block: 16px; - --accordion-fixed-content-padding-inline: 16px; - } + --accordion-fixed-content-padding-block: 16px; + --accordion-fixed-content-padding-inline: 16px; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.scss` around lines 52 - 55, Move the CSS custom property declarations off the universal selector and onto the component container selector to avoid over-broad application: instead of defining --accordion-fixed-content-padding-block and --accordion-fixed-content-padding-inline on *, set them on .bcs-DocGen-collapsible so descendant accordion parts still inherit the values with lower specificity and less risk of accidental overrides.
🤖 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/elements/content-sidebar/DocGenSidebar/TagTree.tsx`:
- Around line 39-42: In the TagTree component (TagTree.tsx) remove interactive
semantics from the fixed leaf label by deleting tabIndex={0}, role="button", and
aria-disabled="true" on that JSX element (the leaf tag label) so it renders as
plain text; if needed, replace with a non-interactive semantic like a span with
role="text" or no role at all and ensure any event handlers/keyboard handlers
tied to that element are also removed or moved to an actual interactive control.
---
Nitpick comments:
In `@src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.scss`:
- Around line 52-55: Move the CSS custom property declarations off the universal
selector and onto the component container selector to avoid over-broad
application: instead of defining --accordion-fixed-content-padding-block and
--accordion-fixed-content-padding-inline on *, set them on
.bcs-DocGen-collapsible so descendant accordion parts still inherit the values
with lower specificity and less risk of accidental overrides.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4a83c13-0633-4b52-b490-6a8ac9860a34
📒 Files selected for processing (3)
src/elements/content-sidebar/DocGenSidebar/DocGenSidebar.scsssrc/elements/content-sidebar/DocGenSidebar/TagTree.tsxsrc/elements/content-sidebar/DocGenSidebar/stories/DocGenSidebar.stories.tsx
| tabIndex={0} | ||
| role="button" | ||
| aria-disabled="true" | ||
| > |
There was a problem hiding this comment.
Remove interactive semantics from non-interactive tag labels.
This creates a focusable, disabled pseudo-button with no action. Use plain text semantics for the fixed leaf label to avoid confusing keyboard/screen-reader navigation.
Suggested fix
<span
className={classNames('bcs-DocGen-tagPath', {
'bcs-DocGen-tagPath--modernized': isPreviewModernizationEnabled,
})}
- tabIndex={0}
- role="button"
- aria-disabled="true"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tabIndex={0} | |
| role="button" | |
| aria-disabled="true" | |
| > | |
| <span | |
| className={classNames('bcs-DocGen-tagPath', { | |
| 'bcs-DocGen-tagPath--modernized': isPreviewModernizationEnabled, | |
| })} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/elements/content-sidebar/DocGenSidebar/TagTree.tsx` around lines 39 - 42,
In the TagTree component (TagTree.tsx) remove interactive semantics from the
fixed leaf label by deleting tabIndex={0}, role="button", and
aria-disabled="true" on that JSX element (the leaf tag label) so it renders as
plain text; if needed, replace with a non-interactive semantic like a span with
role="text" or no role at all and ensure any event handlers/keyboard handlers
tied to that element are also removed or moved to an actual interactive control.
myu-box
left a comment
myu-box
left a comment
There was a problem hiding this comment.
LGTM. verified the style
2 participants
Adjusting tags to have consistent look in according in docgen side pannel, as a request from design team. Had to do padding overwrite as well as text-size because designs are different for old and modernized blueprint components
Summary by CodeRabbit
New Features
Style