Topic style fixes - #199
Conversation
- Updated CSS for drawer component to ensure inset styles are applied - Enhanced tests to verify positioning at viewport edges for 'right' and 'bottom' positions. Signed-off-by: Cory Rylan <crylan@nvidia.com>
- Adjusted the CSS variables for size and gap to enhance layout consistency. - Updated the size property logic in the Select component to correctly reflect the intended size. - Enhanced tests to verify proper rendering of options without overflow and ensure no blank space after visible rows. Signed-off-by: Cory Rylan <crylan@nvidia.com>
📝 WalkthroughWalkthroughThe changes strengthen drawer edge positioning and revise select sizing calculations. Select tests now verify direct size values, visible row boundaries, and the absence of unnecessary scrolling or blank space. ChangesDrawer positioning
Select sizing
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
projects/core/src/drawer/drawer.cssESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. projects/core/src/drawer/drawer.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. projects/core/src/select/select.cssESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/core/src/select/select.css`:
- Line 44: Update the scrollbar-color declaration in the select stylesheet to
use lowercase currentcolor in the color-mix expression, preserving the existing
value and behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 07c1fa1e-c009-4af6-b456-fafe27cb370c
⛔ Files ignored due to path filters (4)
projects/core/.visual/pagination.dark.pngis excluded by!**/*.pngprojects/core/.visual/pagination.pngis excluded by!**/*.pngprojects/core/.visual/select.dark.pngis excluded by!**/*.pngprojects/core/.visual/select.pngis excluded by!**/*.png
📒 Files selected for processing (5)
projects/core/src/drawer/drawer.cssprojects/core/src/drawer/drawer.test.tsprojects/core/src/select/select.cssprojects/core/src/select/select.test.tsprojects/core/src/select/select.ts
| [input] { | ||
| height: var(--height); | ||
| scroll-behavior: smooth; | ||
| scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentColor 20%) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use lowercase currentcolor to satisfy Stylelint.
value-keyword-case reports this declaration. CSS behavior is unchanged, but the current spelling can fail the style check.
Proposed fix
- scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentColor 20%)
+ scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentcolor 20%)📝 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.
| scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentColor 20%) | |
| scrollbar-color: color-mix(in oklab, var(--nve-sys-scrollbar-thumb-color) 100%, currentcolor 20%) |
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 44-44: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/core/src/select/select.css` at line 44, Update the scrollbar-color
declaration in the select stylesheet to use lowercase currentcolor in the
color-mix expression, preserving the existing value and behavior.
Source: Linters/SAST tools
|
🎉 This issue has been resolved in version 2.2.2 🎉 |
|
🎉 This issue has been resolved in version 2.1.2 🎉 |
|
🎉 This issue has been resolved in version 2.3.0 🎉 |
|
🎉 This issue has been resolved in version 2.1.9 🎉 |
Summary by CodeRabbit
Bug Fixes
UI Improvements