-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WEB-5604] fix: lable layout title column and content wrapper styling #8273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe PR contains layout refinements across workspace and spreadsheet components. Left padding behavior in the workspace content wrapper is adjusted based on app rail visibility, and a maximum width constraint is added to the spreadsheet issue row's first column. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx (1)
258-258: Layout constraint looks appropriate.The
max-w-lg(512px) constraint on the sticky first column prevents it from expanding excessively while the existing truncation classes handle overflow properly. Since the column is already responsive withmd:sticky, this change complements the overall layout behavior.Optionally, consider verifying the column width works well across different viewport sizes, especially on larger screens where 512px might feel restrictive if users have very long issue names. You could test with various content lengths to ensure the balance between readability and layout density is optimal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/ce/components/workspace/content-wrapper.tsx(1 hunks)apps/web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsxapps/web/ce/components/workspace/content-wrapper.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/ce/components/workspace/content-wrapper.tsx (1)
26-28: Padding logic is correct.The updated padding behavior properly handles spacing based on AppRail visibility: when the AppRail is not rendered, the content gets
pl-2(8px) to avoid being flush against the edge; when the AppRail is present, padding is removed since the AppRail provides its own spacing. The existing transition classes ensure smooth visual updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes styling issues in the spreadsheet layout's title column and the workspace content wrapper's padding logic. The changes address layout bugs related to the sticky column width constraints and padding behavior when the app rail is visible.
Key Changes:
- Added
max-w-lgconstraint to the spreadsheet issue row's sticky column to prevent excessive width growth - Corrected the workspace content wrapper padding logic to properly handle spacing when the app rail is rendered vs. hidden
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
apps/web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx |
Adds max-w-lg width constraint to the sticky table cell containing issue identifier and title |
apps/web/ce/components/workspace/content-wrapper.tsx |
Inverts padding logic: base pl-2 with pl-0 when app rail renders (previously pl-0 base with pl-2 when app rail renders) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ref={cellRef} | ||
| tabIndex={0} | ||
| className="relative md:sticky left-0 z-10 group/list-block bg-custom-background-100" | ||
| className="relative md:sticky left-0 z-10 group/list-block bg-custom-background-100 max-w-lg" |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding max-w-lg to the sticky table cell may cause width misalignment with the corresponding header cell in spreadsheet-header.tsx (line 48), which has min-w-60 but no max-width constraint. In table layouts, sticky header and body cells should typically have matching width constraints to maintain column alignment. Consider either:
- Adding the same
max-w-lgconstraint to the header cell - Using a more standard approach like a fixed width class on both elements
- Verifying that the table layout still aligns correctly after this change
Description
This PR includes fix for table layout and content wrapper styling.
Type of Change
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.