adjust editors styles in full screen mode#1921
Conversation
Removed unnecessary class properties from DialogContent.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3d880ef The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@claude --review and add a changeset |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
0 Key Findings | Risk: Low
💭 Consider (1) 💭
- 💭 Consider:
editor.tsx:27Fullscreen variant base style constraints — thesize="fullscreen"variant may not fully override base DialogContent styles
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
expandable-field.tsx:40 |
overflow-x-auto without keyboard navigation hint for scrollable regions |
LOW confidence, edge case speculation — horizontal overflow is unlikely in typical label usage |
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-frontend |
2 | 0 | 0 | 0 | 1 | 0 | 1 |
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 2 | 0 | 0 | 0 | 1 | 0 | 1 |
✅ APPROVE
Summary: Clean CSS refactoring that simplifies fullscreen editor styling by relying on the size="fullscreen" variant. The changes are focused and coherent. One minor consideration flagged about whether the fullscreen variant fully overrides base styles, but this is likely intentional given the author is specifically working on fullscreen styling. The layout improvements in ExpandableField (flex layout, overflow handling, shrink prevention) are sensible improvements. Ship it! 🚀
| size="fullscreen" | ||
| className="!max-w-none h-screen w-screen max-h-screen p-0 gap-0 border-0 rounded-none" | ||
| > | ||
| <DialogContent size="fullscreen" className="duration-0"> |
There was a problem hiding this comment.
💭 Consider: Fullscreen variant base style constraints
Issue: The size="fullscreen" variant in dialog.tsx only adds w-screen h-screen, but doesn't override the base styles (max-w-[calc(100%-2rem)], max-h-[calc(100dvh-1.5rem)], rounded-lg, border, p-6, gap-4).
Why: The previous code used !max-w-none (with Tailwind's important modifier) to force-override these constraints. Without it, max-width takes precedence over width in CSS, so the dialog may not be truly fullscreen.
Fix: If you've visually confirmed this works as intended, this can be dismissed. Otherwise, consider either:
- Adding the overrides to the
fullscreenvariant indialog.tsx:fullscreen: 'w-screen h-screen max-w-none max-h-screen p-0 gap-0 border-0 rounded-none' - Or keeping the critical overrides here in the className
Refs:
Co-authored-by: Dimitri POSTOLOV <dimaMachina@users.noreply.github.com>
There was a problem hiding this comment.
PR Review Summary
0 Key Findings | Risk: Low
Summary
This is a clean CSS refactoring PR that simplifies editor component styling for fullscreen mode:
-
editor.tsx: Removes redundant inline CSS overrides (
!max-w-none h-screen w-screen max-h-screen p-0 gap-0 border-0 rounded-none) that are now handled by thesize="fullscreen"variant in the baseDialogContentcomponent. Addsduration-0to disable transitions for snappier fullscreen toggling. -
expandable-field.tsx: Restructures layout from margin-based spacing (
space-y-2) to flexbox-based spacing (gap-2 h-full flex flex-col). Simplifies the children wrapper and addsoverflow-x-autofor better horizontal scroll handling.
Both changes appear intentional and follow modern CSS/flexbox patterns. The code leverages existing component variants rather than manual overrides, improving maintainability.
🟡 Minor (0) 🟡
No issues identified.
💭 Consider (0) 💭
No items.
✅ APPROVE
Summary: Clean CSS simplification that properly leverages the existing size="fullscreen" dialog variant. The flexbox refactoring in ExpandableField improves layout consistency. No functional or visual regressions identified. Ship it! 🚀
Discarded (0)
No items discarded.
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-frontend |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
There was a problem hiding this comment.
PR Review Summary
0 Key Findings | Risk: Low
Re-review — no new commits since last automated review at 3d880efd.
🕐 Pending Recommendations (1)
- 💭
editor.tsx:27Fullscreen variant may not fully override base DialogContent styles (max-w-*,max-h-*, etc.)
Discarded (0)
No items discarded.
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: No reviewers dispatched — re-review with no new commits.
✅ APPROVE
Summary: No changes since the last review. The prior APPROVE recommendation stands. The one Consider-level issue about fullscreen variant styling has been flagged in an existing comment and remains for the author to address or dismiss. Clean CSS simplification — ready to ship! 🚀
1 participant
No description provided.