-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix(rich-input.tsx): replace default scrollbar with themed scrollbar in Summary Editor, fixed issue #2420 #2421
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
base: main
Are you sure you want to change the base?
Conversation
…in Summary Editor, fixed AmruthPillai#2420
📝 WalkthroughWalkthroughRefactors the rich-input component's styling hierarchy by extracting scroll and border styling from EditorContent into a new ScrollArea wrapper, moving container properties to the ScrollArea level while adjusting EditorContent's utility classes accordingly. No functional or API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes The change is contained to a single file with straightforward styling and layout adjustments. Review effort focuses on verifying the visual hierarchy is preserved, scrolling behavior remains intact, and no functional regressions are introduced. 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 |
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/ui/src/components/rich-input.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
libs/ui/src/components/rich-input.tsx (2)
libs/ui/src/components/scroll-area.tsx (1)
ScrollArea(5-40)libs/utils/src/namespaces/style.ts (1)
cn(14-14)
🪛 ESLint
libs/ui/src/components/rich-input.tsx
[error] 1-50: Run autofix to sort these imports!
(simple-import-sort/imports)
🔇 Additional comments (2)
libs/ui/src/components/rich-input.tsx (2)
50-50: LGTM!The ScrollArea import is correctly added and necessary for the themed scrollbar implementation.
501-501: Border and padding appropriately moved to ScrollArea.The migration of border and padding styles from EditorContent to the ScrollArea wrapper is correct and aligns with the themed scrollbar implementation.
| attributes: { | ||
| class: cn( | ||
| "prose prose-sm prose-zinc max-h-[200px] max-w-none overflow-y-scroll dark:prose-invert focus:outline-none [&_*]:my-2", | ||
| "prose prose-sm prose-zinc max-h-[200px] max-w-none dark:prose-invert focus:outline-none [&_*]:my-2", |
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.
Height constraint prevents ScrollArea from functioning.
The max-h-[200px] on the prose element (contenteditable) prevents the ScrollArea wrapper from handling overflow and providing themed scrolling. When content exceeds 200px, the prose element itself will limit the height without an overflow property, potentially cutting off content rather than triggering ScrollArea's scroll behavior.
To fix this, move the height constraint from the prose element to the ScrollArea level:
class: cn(
- "prose prose-sm prose-zinc max-h-[200px] max-w-none dark:prose-invert focus:outline-none [&_*]:my-2",
+ "prose prose-sm prose-zinc max-w-none dark:prose-invert focus:outline-none [&_*]:my-2",
editorClassName,
),And update the ScrollArea to include the height constraint:
- <ScrollArea orientation="vertical" className="rounded-sm border px-3 py-2">
+ <ScrollArea orientation="vertical" className="max-h-[200px] rounded-sm border px-3 py-2">This ensures that:
- The prose content can grow naturally beyond 200px
- The ScrollArea container constrains the visible height to 200px
- The themed scrollbar appears when content overflows
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In libs/ui/src/components/rich-input.tsx around line 478, the prose element
currently has the class "max-h-[200px]" which prevents the ScrollArea from
managing overflow; remove "max-h-[200px]" from the prose/contenteditable element
and instead apply the height constraint to the ScrollArea wrapper (e.g., add
"max-h-[200px]" or an equivalent height class to the ScrollArea container or its
scroll viewport) so the inner prose can grow naturally while the ScrollArea
constrains visible height and provides themed scrolling.
| className={cn( | ||
| "grid min-h-[160px] w-full bg-transparent text-sm placeholder:opacity-80 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-primary disabled:cursor-not-allowed disabled:opacity-50", | ||
| hideToolbar && "pt-2", |
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.
🧩 Analysis chain
Verify focus-visible styles on EditorContent.
The focus-visible:outline-none and focus-visible:ring-1 styles are applied to the EditorContent container, but the actual focusable element is the contenteditable prose element inside. These focus styles may not trigger as expected.
Run the following script to check if there are other focus styles defined for the inner prose element or if focus delegation is handled elsewhere:
🏁 Script executed:
#!/bin/bash
# Search for focus-related styles in TipTap configuration and related files
rg -nP --type=tsx --type=ts -C3 'focus.*outline|outline.*focus|focus.*ring|ring.*focus' -g '!**/node_modules/**'Length of output: 27
I'll rerun the search with corrected ripgrep syntax. The issue is that ripgrep uses different type names; let me search using file glob patterns instead:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for focus-related styles using glob patterns
rg -nP -C3 'focus.*outline|outline.*focus|focus.*ring|ring.*focus' --glob='*.tsx' --glob='*.ts'
Length of output: 22898
🏁 Script executed:
#!/bin/bash
# Also check the rich-input.tsx file directly to understand structure
rg -nP -C5 'EditorContent|contenteditable|prose' libs/ui/src/components/rich-input.tsxLength of output: 2355
🏁 Script executed:
#!/bin/bash
# Search for TipTap-related focus configurations
rg -nP -C3 'TipTap|Editor\(' --glob='*.tsx' --glob='*.ts' | head -100Length of output: 492
🏁 Script executed:
#!/bin/bash
# Look for focus event handlers or refs in the component
rg -nP -C3 'onFocus|onBlur|ref.*focus' libs/ui/src/components/rich-input.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are any CSS/style files related to EditorContent
fd -e css -e scss -e less | xargs rg -l 'EditorContent|prose' 2>/dev/nullLength of output: 33
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the CSS file for prose-related focus styles
cat apps/artboard/src/styles/main.css | grep -A 5 -B 5 'prose\|focus'
Length of output: 403
🏁 Script executed:
#!/bin/bash
# Look for how EditorContent component might delegate focus or handle focus styling
rg -nP 'EditorContent|\.ProseMirror' --glob='*.tsx' --glob='*.ts' --glob='*.css' -C 2Length of output: 1408
🏁 Script executed:
#!/bin/bash
# Check if there's any global prose styling that handles focus
rg -nP 'prose.*focus|focus.*prose' --glob='*.css' --glob='*.scss'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Verify TipTap prosemirror focus behavior - check for any refs or focus management
rg -nP 'useEditor|prosemirror|contenteditable' libs/ui/src/components/rich-input.tsx -A 10 -B 2Length of output: 1028
Apply focus styles to the inner contenteditable element, not the EditorContent container.
The focus-visible styles on the EditorContent wrapper (line 505) never trigger because the container itself is not focusable. Focus lands on the inner contenteditable element, which only has focus:outline-none with no alternative focus ring. Move the focus styling to editorProps.attributes (line 478) so it applies to the actual focusable prose element.
Lines to fix:
- Line 478: Add
focus:ring-1 focus:ring-primaryto the prose element - Line 505: Remove
focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-primaryfrom the container
🤖 Prompt for AI Agents
In libs/ui/src/components/rich-input.tsx around lines 478 and 504-506, the
focus-visible styles are applied to the EditorContent wrapper which is not
focusable; move those focus-ring classes to the actual contenteditable element:
add "focus:ring-1 focus:ring-primary" (and keep "focus:outline-none") to the
prose element via editorProps.attributes at line 478, and remove
"focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-primary"
from the EditorContent container classes at lines 504-506 so the visible focus
ring appears on the real focus target.
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
Replace the default scrollbar in the Summary editor with a themed scrollbar by wrapping the editor content in a ScrollArea.
- Wrap EditorContent with ScrollArea to apply themed scrollbar
- Move border/padding styling from EditorContent to ScrollArea and remove overflow-y-scroll from editor content classes
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| attributes: { | ||
| class: cn( | ||
| "prose prose-sm prose-zinc max-h-[200px] max-w-none overflow-y-scroll dark:prose-invert focus:outline-none [&_*]:my-2", | ||
| "prose prose-sm prose-zinc max-h-[200px] max-w-none dark:prose-invert focus:outline-none [&_*]:my-2", |
Copilot
AI
Oct 18, 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.
Removing overflow-y-scroll but keeping max-h-[200px] on the inner editor content caps its height without making it scrollable; the overflow will not be managed by the ScrollArea. Move the height constraint to the ScrollArea wrapper (or its viewport) and remove max-h-[200px] here so the content can grow and the ScrollArea can handle scrolling.
| {...props} | ||
| /> | ||
|
|
||
| <ScrollArea orientation="vertical" className="rounded-sm border px-3 py-2"> |
Copilot
AI
Oct 18, 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.
The ScrollArea wrapper lacks a height constraint, so it won’t create a scrollable viewport. Add max-h-[200px] (moved from the inner editor) to the ScrollArea and let the child content grow; for example:
| "grid min-h-[160px] w-full bg-transparent text-sm placeholder:opacity-80 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-primary disabled:cursor-not-allowed disabled:opacity-50", | ||
| hideToolbar && "pt-2", | ||
| className, |
Copilot
AI
Oct 18, 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.
[nitpick] className is still applied to EditorContent, but the visible border/padding moved to the ScrollArea wrapper. This changes how consumers style the component and may be a breaking change (styling the border no longer works via className). For backward compatibility, consider applying className to the ScrollArea (e.g., className={cn("max-h-[200px] rounded-sm border px-3 py-2", className)}) and, if needed, introduce a separate prop (e.g., contentClassName) for EditorContent.
|
@AmruthPillai can I make changes based on Copilot response? |
Old Summary editor with default scrollbar
New themed scrollbar added ✅
Summary by CodeRabbit
Refactor
Style