fix: fixed codebox height for active tab (#8425) #8450
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
There was a problem hiding this comment.
Pull request overview
This PR addresses a variable height issue in code blocks by applying a fixed height of 19rem to code blocks within active tabs. The fix targets the BaseCodeBox component when rendered inside tabs with an active state.
- Introduces a fixed height constraint for code blocks in active tabs
- Uses CSS selector targeting
div[data-state='active']to scope the height change
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| div[data-state='active'] .root pre { | ||
| height: 19rem; | ||
| } |
There was a problem hiding this comment.
The CSS selector is overly broad and may cause unintended side effects. It targets any div with data-state='active' followed by any descendant with class 'root' and a pre element. This could affect other components in the codebase that use similar patterns. Consider using a more specific selector by adding a unique class to the CodeTabs or Tabs component wrapper, or scope this selector to be more specific to the BaseCodeBox component within a tabs context.
| } | ||
|
|
||
| div[data-state='active'] .root pre { | ||
| height: 19rem; |
There was a problem hiding this comment.
The fixed height of 19rem (304px) is a magic number without documentation or explanation. Consider extracting this value to a CSS custom property (CSS variable) with a descriptive name, or adding a comment explaining why this specific height was chosen. Additionally, this fixed height may cause issues on different screen sizes or with varying amounts of code content. Consider whether overflow-y: auto should be added to handle cases where content exceeds the fixed height, or whether a max-height with min-content behavior would be more appropriate.
| @apply size-4; | ||
| } | ||
|
|
||
| div[data-state='active'] .root pre { |
There was a problem hiding this comment.
Setting a fixed height on the pre element without overflow-y handling will cause content that exceeds 19rem to be cut off and inaccessible to users. The existing overflow-x-auto is on the nested code element (line 19), not the pre element. Consider adding overflow-y-auto to this rule to ensure users can scroll through longer code blocks, or verify that all code content in active tabs will always fit within 19rem.
| div[data-state='active'] .root pre { | |
| div[data-state='active'] .root pre { | |
| @apply overflow-y-auto; |
avivkeller
left a comment
avivkeller
left a comment
There was a problem hiding this comment.
Don't we just need to make all the code boxes the same height, without CSS changes?
2 participants
Description
This PR fixes the variable height issue of code block by introducing fixed height for the code block
Validation
Screen.Recording.2025-12-25.161204.mp4
Related Issues
Fixes #8425
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.