Fix mobile correlation control layout - #2320
Conversation
Keep full metric and lag labels readable at narrow viewports.\n\nRefs #2148
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
Reviewer's GuideMakes the correlation controls responsive by stacking metric selectors and wrapping lag options on small web viewports, updates tests and Storybook to cover the new layout, and documents the implementation plan. Flow diagram for responsive correlation controls layoutflowchart LR
A[Viewport width]
A -->|< sm| B[Metric controls use grid
stacked X and Y]
A -->|>= sm| C[Metric controls use grid
row X vs Y]
B --> D[Lag options in flex
with flex-wrap]
C --> D
D --> E[Comparison sentence in
separate block below
lag options]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Summary by QodoFix responsive web layout for correlation controls
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
|
Storybook previews for This comment updates automatically on each PR push. |
Code Review by Qodo
Context used✅ Compliance rules (platform):
167 rules✅ Skills:
fix-provider, write-tests, cloudflare 1. Brittle class substring assertions
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| const xAxisSelect = screen.getByLabelText("X axis"); | ||
| const metricControls = xAxisSelect.parentElement?.parentElement; | ||
| expect(metricControls?.className).toContain("grid"); | ||
| expect(metricControls?.className).toContain("sm:grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)]"); | ||
|
|
||
| const sameDayButton = screen.getByRole("button", { name: "Same day" }); | ||
| const lagOptions = sameDayButton.parentElement; | ||
| expect(lagOptions?.className).toContain("flex-wrap"); |
There was a problem hiding this comment.
2. Brittle class substring assertions 🐞 Bug ⚙ Maintainability
The updated CorrelationExplorerPage test asserts layout using className.toContain("grid") /
toContain("flex-wrap"), which can false-pass if only variant-prefixed utilities exist (e.g.
sm:grid, sm:flex-wrap) and therefore doesn’t reliably verify the mobile-first layout.
Agent Prompt
## Issue description
The test uses substring checks on `className` for Tailwind utilities. Substring matching can pass even when the exact class token is missing (e.g., `"grid"` matches `"sm:grid-cols-..."`), weakening the regression coverage for the intended mobile-first classes.
## Issue Context
This repo already loads `@testing-library/jest-dom/vitest`, so `toHaveClass` is available and will assert exact class tokens.
## Fix Focus Areas
- packages/web/src/pages/CorrelationExplorerPage.test.tsx[381-407]
- packages/web/test-setup.ts[1-2]
## Proposed fix
- Replace:
- `expect(metricControls?.className).toContain("grid")`
- `expect(lagOptions?.className).toContain("flex-wrap")`
with token-aware assertions, e.g.:
- `expect(metricControls).toHaveClass("grid")`
- `expect(metricControls).toHaveClass("sm:grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)]")`
- `expect(lagOptions).toHaveClass("flex-wrap")`
- Also avoid optional chaining in the assertion target after locating the element; assert it’s non-null first so failures are clearer.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Summary
The audit issue title says mobile, but the reproduced defect is the responsive web surface. Native mobile already uses the intended stacked structure and remains unchanged. A simulator audit was not applicable because the changed surface is web; generating native infrastructure and API/auth state would not validate this defect.
Validation
pnpm exec vitest run packages/web/src/pages/CorrelationExplorerPage.test.tsx --project unit(11 passed)pnpm exec vitest run packages/mobile/app/correlation.test.tsx --project mobile(12 passed)pnpm test:changed(18 passed)pnpm typecheckpnpm lint(all gates passed; analytics lint used the workspace-scoped ClickHouse port)pnpm storybook:web:buildpnpm storybook:mobile:buildFixes #2148
Summary by Sourcery
Improve the responsive layout of the correlation controls on the web correlation explorer page to prevent overflow on narrow viewports while preserving the desktop arrangement.
Enhancements:
Documentation:
Summary by cubic
Stacks the correlation metric selects and wraps lag options on narrow web viewports to prevent overflow, while preserving the desktop row layout. Adds a responsive regression test and a mobile viewport Storybook; fixes #2148.
smwithsm:grid-cols-[minmax(0,1fr)_auto_minmax(0,1fr)]; "vs" hidden on small.flex-wrapand moved the comparison sentence into its own block below.Written for commit 5bc2430. Summary will update on new commits.