Conversation
|
WalkthroughThe CodeBlock component adds a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Fixed modal text overflowing
|
|
||
| {shouldHighlight ? ( | ||
| <HighlightCode | ||
| theme={theme} | ||
| code={code} | ||
| language={language} | ||
| showLineNumbers={showLineNumbers} | ||
| highlightLines={highlightLines} | ||
| maxLineWidth={maxLineWidth} | ||
| className="min-h-full" | ||
| preClassName="text-sm" | ||
| isWrapped={isWrapped} | ||
| /> | ||
| ) : ( | ||
| <div | ||
| dir="ltr" | ||
| className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600" | ||
| > | ||
| <pre className="relative mr-2 p-2 font-mono text-base leading-relaxed" dir="ltr"> | ||
| {highlightSearchText(code, searchTerm)} | ||
| </pre> | ||
| </div> | ||
| )} | ||
| <div onMouseDown={handleCodeMouseDown} className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600"> | ||
| {shouldHighlight ? ( | ||
| <HighlightCode | ||
| theme={theme} | ||
| code={code} | ||
| language={language} | ||
| showLineNumbers={showLineNumbers} | ||
| highlightLines={highlightLines} | ||
| maxLineWidth={maxLineWidth} | ||
| className="min-h-full" | ||
| preClassName="text-sm" | ||
| isWrapped={isWrapped} | ||
| /> | ||
| ) : ( | ||
| <div | ||
| dir="ltr" | ||
| className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600" | ||
| > | ||
| <pre className="relative mr-2 p-2 font-mono text-base leading-relaxed" dir="ltr"> | ||
| {highlightSearchText(code, searchTerm)} | ||
| </pre> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🟡 Modal code block has duplicated padding and scrollbar classes causing double padding
In the modal's shouldHighlight path, the new wrapper <div> at line 417 applies overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600, but the HighlightCode component internally already renders its own container with px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600 plus overflow classes (see containerClasses at apps/webapp/app/components/code/CodeBlock.tsx:515-520). This results in double px-3 py-3 padding and nested scroll containers in the modal.
Root Cause
Before this PR, the shouldHighlight branch in the modal rendered <HighlightCode> directly as a child of <DialogContent>, and the non-highlighted branch had its own wrapper <div> with the scrollbar/padding classes. The PR moved both branches inside a new wrapper <div> that carries the scrollbar/padding classes from the old non-highlighted wrapper, but the HighlightCode component already has these same classes internally:
// New outer wrapper (line 417-418)
<div onMouseDown={handleCodeMouseDown} className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600">
// HighlightCode internally renders (line 515-520):
// <div className="px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600 overflow-x-auto min-h-full">Similarly, for the !shouldHighlight branch, the original inner <div> at line 432-434 still has overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600, creating double nesting of the same styles.
Impact: The modal code block will have double padding (px-3 py-3 applied twice = 1.5rem on each side instead of 0.75rem), and nested scroll containers which can cause confusing scroll behavior.
Prompt for agents
In apps/webapp/app/components/code/CodeBlock.tsx, the new wrapper div at line 417 that was added for the mouseDown handler has className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600". This duplicates the classes that already exist inside both the HighlightCode component (lines 515-520) and the non-highlighted inner div (lines 432-434).
To fix this:
1. On line 417-418, remove the duplicate classes from the outer wrapper div, keeping only the onMouseDown handler. Change it to: <div onMouseDown={handleCodeMouseDown}>
2. For the non-highlighted path (lines 431-439), the inner div at line 432-434 already has the correct overflow/padding/scrollbar classes, so it will continue to work correctly.
3. For the highlighted path, HighlightCode already has its own container classes internally, so it will also work correctly.
4. However, you may need to keep overflow-auto on the outer wrapper for the modal to scroll properly. In that case, just remove the padding classes (px-3 py-3) and scrollbar classes from the outer wrapper, and ensure the inner components handle their own padding.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| {shouldHighlight ? ( | ||
| <HighlightCode | ||
| theme={theme} | ||
| code={code} | ||
| language={language} | ||
| showLineNumbers={showLineNumbers} | ||
| highlightLines={highlightLines} | ||
| maxLineWidth={maxLineWidth} | ||
| className="min-h-full" | ||
| preClassName="text-sm" | ||
| isWrapped={isWrapped} | ||
| /> | ||
| ) : ( | ||
| <div | ||
| dir="ltr" | ||
| className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600" | ||
| > | ||
| <pre className="relative mr-2 p-2 font-mono text-base leading-relaxed" dir="ltr"> | ||
| {highlightSearchText(code, searchTerm)} | ||
| </pre> | ||
| </div> | ||
| )} | ||
| <div onMouseDown={handleCodeMouseDown} className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600"> | ||
| {shouldHighlight ? ( | ||
| <HighlightCode | ||
| theme={theme} | ||
| code={code} | ||
| language={language} | ||
| showLineNumbers={showLineNumbers} | ||
| highlightLines={highlightLines} | ||
| maxLineWidth={maxLineWidth} | ||
| className="min-h-full" | ||
| preClassName="text-sm" | ||
| isWrapped={isWrapped} | ||
| /> | ||
| ) : ( | ||
| <div | ||
| dir="ltr" | ||
| className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600" | ||
| > | ||
| <pre className="relative mr-2 p-2 font-mono text-base leading-relaxed" dir="ltr"> | ||
| {highlightSearchText(code, searchTerm)} | ||
| </pre> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🚩 Modal HighlightCode no longer receives overflow-auto directly — outer wrapper changes scroll behavior
Before this PR, in the modal the HighlightCode component was rendered directly inside DialogContent, and its internal container provided the overflow scrolling. Now, the new outer wrapper at line 417 has overflow-auto, which means the outer div will be the scroll container for the modal content. The HighlightCode internal container also has overflow-x-auto or overflow-y-auto depending on isWrapped. Having nested scroll containers could lead to unexpected scroll behavior — particularly the inner container may not scroll as expected if the outer one captures the scroll. This is related to but distinct from the duplicate padding bug already reported.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/code/CodeBlock.tsx`:
- Around line 418-441: The outer wrapper in the CodeBlock component currently
duplicates scrollbar/padding styles; remove the duplicated layout classes from
the outer <div> that has onMouseDown (leave only the onMouseDown handler and no
padding/scroll classes) so the inner components own scrolling/padding. Keep the
existing scrollbar/padding/overflow classes on HighlightCode and the fallback
inner <div> that renders highlightSearchText, ensuring there is a single scroll
container and no doubled px-3 py-3 or nested overflow-auto.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/code/CodeBlock.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/components/code/CodeBlock.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/code/CodeBlock.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/components/code/CodeBlock.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/code/CodeBlock.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/components/code/CodeBlock.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/components/code/CodeBlock.tsx
🧠 Learnings (1)
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/code/CodeBlock.tsx
🧬 Code graph analysis (1)
apps/webapp/app/components/code/CodeBlock.tsx (4)
packages/core/src/v3/apiClient/stream.ts (1)
e(216-222)packages/cli-v3/src/executions/taskRunProcess.ts (1)
code(342-404)apps/webapp/app/utils/cn.ts (1)
cn(77-79)apps/webapp/app/utils/logUtils.ts (1)
highlightSearchText(27-70)
⏰ 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). (25)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Cloudflare Workers
🔇 Additional comments (2)
apps/webapp/app/components/code/CodeBlock.tsx (2)
220-241: LGTM — selection-limiting logic is well structured.The
restoreRef+handleCodeMouseDown+ cleanupuseEffectproperly handles all edge cases: left-click gating,{ once: true }on both listeners, cross-removal of the surviving listener inrestore, and unmount cleanup. This addresses the prior feedback cleanly.
360-397: LGTM — main view wrapping is clean.The outer
<div>correctly carries only theonMouseDownhandler, delegating all layout/scroll concerns toHighlightCodeand the fallback container respectively.
| <div onMouseDown={handleCodeMouseDown} className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600"> | ||
| {shouldHighlight ? ( | ||
| <HighlightCode | ||
| theme={theme} | ||
| code={code} | ||
| language={language} | ||
| showLineNumbers={showLineNumbers} | ||
| highlightLines={highlightLines} | ||
| maxLineWidth={maxLineWidth} | ||
| className="min-h-full" | ||
| preClassName="text-sm" | ||
| isWrapped={isWrapped} | ||
| /> | ||
| ) : ( | ||
| <div | ||
| dir="ltr" | ||
| className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600" | ||
| > | ||
| <pre className="relative mr-2 p-2 font-mono text-base leading-relaxed" dir="ltr"> | ||
| {highlightSearchText(code, searchTerm)} | ||
| </pre> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Double padding and nested scroll containers in the modal.
The outer wrapper (line 418) applies overflow-auto px-3 py-3 scrollbar-thin …, but both inner paths duplicate those same styles:
- Highlighted path:
HighlightCode's internal container (lines 515–520) also appliespx-3 py-3 scrollbar-thin … overflow-x-auto. - Non-highlighted path: The inner
<div>at line 432–434 repeats the identical classes verbatim.
This results in ~px-6-equivalent horizontal padding and nested scrollable regions. Compare with the main view (line 360), where the outer <div> carries only onMouseDown and leaves styling to the children.
Suggested fix: move scroll/padding styling to children only
- <div onMouseDown={handleCodeMouseDown} className="overflow-auto px-3 py-3 scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600">
+ <div onMouseDown={handleCodeMouseDown} className="overflow-auto">
{shouldHighlight ? (
<HighlightCode
theme={theme}This keeps the outer wrapper as a simple scroll host (or just the mousedown handler, mirroring the main view), letting HighlightCode and the fallback <div> own their own padding and scrollbar styling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/components/code/CodeBlock.tsx` around lines 418 - 441, The
outer wrapper in the CodeBlock component currently duplicates scrollbar/padding
styles; remove the duplicated layout classes from the outer <div> that has
onMouseDown (leave only the onMouseDown handler and no padding/scroll classes)
so the inner components own scrolling/padding. Keep the existing
scrollbar/padding/overflow classes on HighlightCode and the fallback inner <div>
that renders highlightSearchText, ensuring there is a single scroll container
and no doubled px-3 py-3 or nested overflow-auto.
1 participant
✅ Checklist
Testing
Manually tested mouse text selection
Changelog
Fixed issue where mouse text election in code blocks would select other elements of the page.
Screenshots
Screen.Recording.2026-02-17.at.00.14.53.mov
💯