fix(playground): prevent model group option stretching#6120
Open
QuentinHsu wants to merge 2 commits into
Open
Conversation
- keep group options at a fixed 2rem height and align them to the top. - organize layout code in a dedicated module and cover layout and scrolling behavior.
- require regression coverage for behavior changes, bug fixes, and UI states. - define module-scoped test organization, stable assertions, mocks, and verification rules.
Contributor
WalkthroughTesting guidance was expanded, while the model group selector received a nested layout import, fixed grid-row styling, and tests for layout classes and option scrolling behavior. ChangesTesting guidance
Model group selector layout
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
web/default/src/components/model-group-selector/__tests__/layout.test.ts (1)
19-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Vitest instead of
node:testfor unit tests in__tests__/directories.The coding guidelines for
web/default/**/__tests__/**/*.test.tsspecify that unit tests for utility functions and pure logic should use Vitest. This file imports fromnode:testandnode:assert/strictinstead. Additionally, AGENTS.md line 147 states "工具函数与纯逻辑优先单元测试（Vitest）" and line 164 says "新增或大幅重写测试时优先使用 Vitest 与 React Testing Library 的标准断言和查询方式".The test logic itself is correct and well-structured — the scroll centering math (scrollTop = 76) and the
scrollIntoViewfallback assertion are both accurate.♻️ Proposed refactor: switch to Vitest
-import assert from 'node:assert/strict' -import { describe, test } from 'node:test' +import { describe, it, expect } from 'vitest' import { modelGroupSelectorLayoutClasses, scrollSelectedOptionIntoView, } from '../layout' describe('model group selector layout', () => { - test('keeps group options at a fixed height and aligned to the top', () => { + it('keeps group options at a fixed height and aligned to the top', () => { const groupScrollClasses = modelGroupSelectorLayoutClasses.groupScroll.split(' ') - assert.ok(groupScrollClasses.includes('auto-rows-[2rem]')) - assert.ok(groupScrollClasses.includes('content-start')) + expect(groupScrollClasses).toContain('auto-rows-[2rem]') + expect(groupScrollClasses).toContain('content-start') }) - test('centers the selected group inside its own scroll container', () => { + it('centers the selected group inside its own scroll container', () => { const scrollCalls: ScrollToOptions[] = [] const selectedOption = { offsetHeight: 32, offsetTop: 160, scrollIntoView() {}, } const scrollContainer = { clientHeight: 200, scrollTop: 0, scrollTo(options: ScrollToOptions) { scrollCalls.push(options) }, } scrollSelectedOptionIntoView(selectedOption, scrollContainer) - assert.deepEqual(scrollCalls, [{ top: 76, behavior: 'auto' }]) + expect(scrollCalls).toEqual([{ top: 76, behavior: 'auto' }]) }) - test('falls back to scrollIntoView when no group container is provided', () => { + it('falls back to scrollIntoView when no group container is provided', () => { const scrollCalls: ScrollIntoViewOptions[] = [] const selectedOption = { scrollIntoView(options?: ScrollIntoViewOptions) { scrollCalls.push(options ?? {}) }, } scrollSelectedOptionIntoView(selectedOption) - assert.deepEqual(scrollCalls, [{ block: 'center', inline: 'nearest' }]) + expect(scrollCalls).toEqual([{ block: 'center', inline: 'nearest' }]) }) })Also applies to: 28-34
🤖 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 `@web/default/src/components/model-group-selector/__tests__/layout.test.ts` around lines 19 - 20, Update the tests in this file to use Vitest instead of node:test and node:assert/strict: replace the describe/test imports with Vitest equivalents and migrate assertions to Vitest assertions, while preserving the existing scroll-centering and scrollIntoView fallback test behavior.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@web/default/src/components/model-group-selector/__tests__/layout.test.ts`:
- Around line 19-20: Update the tests in this file to use Vitest instead of
node:test and node:assert/strict: replace the describe/test imports with Vitest
equivalents and migrate assertions to Vitest assertions, while preserving the
existing scroll-centering and scrollIntoView fallback test behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94ebb525-f031-410f-8aed-6acd6113d3de
📒 Files selected for processing (4)
web/default/AGENTS.mdweb/default/src/components/model-group-selector.tsxweb/default/src/components/model-group-selector/__tests__/layout.test.tsweb/default/src/components/model-group-selector/layout.ts
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
1 participant
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.
Important
📝 变更描述 / Description
问题描述
修复方式
2rem，并使用顶部对齐，避免选项被剩余空间拉伸。model-group-selector专属模块。__tests__目录，避免测试文件与正式代码平铺。测试说明
scrollIntoView降级处理。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
bun test：36 项测试全部通过。bun run typecheck：通过。oxlint与oxfmt --check：通过。bun run build：生产构建通过。Summary by CodeRabbit
Bug Fixes
Tests