From a82c7e37e7001d4091fc33da979ac2d7436a22f1 Mon Sep 17 00:00:00 2001 From: Louis Knight-Webb Date: Mon, 8 Dec 2025 17:34:12 +0000 Subject: [PATCH] GitHub comment dialog overflow issue (vibe-kanban) (#1465) * All changes are complete and tests pass. Here's a summary of what was done: ## Summary Fixed the GitHub Comments Dialog overflow issue with two files modified: ### 1. `frontend/src/components/ui/github-comment-card.tsx` - Made the layout classes (`inline-block`, `align-bottom`, `max-w-md`) conditional - They only apply when no custom layout className is provided (checked for `flex-`, `block`, or `w-` in className) - This preserves the WYSIWYG editor behavior while allowing the dialog to override ### 2. `frontend/src/components/dialogs/tasks/GitHubCommentsDialog.tsx` - Added `min-w-0` to the scrollable content container (line 108) - Added `min-w-0` to each comment row flex container (line 145) - Simplified the GitHubCommentCard className to `"flex-1 min-w-0"` (line 175) The `min-w-0` classes are essential for proper flex containment - they override the default `min-width: auto` behavior that prevents flex items from shrinking below their content size. Both TypeScript type checking and ESLint pass successfully. * All changes are complete and tests pass. Here's a summary of what was fixed: ## Summary **Root cause**: The Dialog component's API was being misused. Width classes (`max-w-2xl`) were being passed to `DialogContent` instead of `Dialog`. Since `DialogContent` is nested inside the Dialog's inner container (which has default `max-w-lg`), the width override was ineffective. **Changes made**: 1. **`GitHubCommentsDialog.tsx`**: Moved sizing classes from `DialogContent` to `Dialog`: - `Dialog` now has `className="max-w-2xl p-0 max-h-[80vh] overflow-hidden"` - `DialogContent` now has `className="flex flex-col h-full"` 2. **`github-comment-card.tsx`**: Added `overflow-hidden` to the FullCard container so inner elements (DiffHunk `
`) can scroll properly within the card boundaries.

The dialog should now:
- Be properly sized at `max-w-2xl` (672px) instead of being constrained to `max-w-lg` (512px)
- Properly contain content without horizontal overflow
- Allow diff hunks to scroll horizontally within the cards

* All changes are complete and tests pass. Summary of the fix for vertical scrolling:

1. **`dialog.tsx`**: Changed `grid` to `flex flex-col` in the Dialog inner container - this enables proper flex-based height constraints for child elements

2. **`GitHubCommentsDialog.tsx`**: Added `min-h-0` to the scrollable content div - this overrides the default `min-height: auto` which was preventing the flex item from shrinking below its content size

The dialog should now scroll vertically when there are many comments.

* All changes are complete and tests pass. Here's a summary of what was done:

**Changes to fix vertical scrolling:**

The dialog now follows the same pattern as `ViewProcessesDialog`:

1. **Dialog**: `className="max-w-2xl p-0 overflow-hidden"` - removed height constraints
2. **DialogContent**: `className="p-0"` - removed `h-full` and flex classes
3. **Added wrapper div**: `className="max-h-[70vh] flex flex-col min-h-0"` - explicit height constraint on inner wrapper
4. **Scrollable content**: `className="p-4 overflow-auto flex-1 min-h-0"` - scrolls within the constrained wrapper

The key insight was that putting the height constraint (`max-h-[70vh]`) on an **inner wrapper** rather than on the Dialog itself makes the flex-based scrolling work correctly. The header and footer remain outside this wrapper so they stay fixed.

* Cleanup script changes for task attempt a7065bc4-aa62-4dab-90c7-d15524acc50f

* Done! The hacky string matching has been replaced with an explicit `layout` prop:

**Changes:**

1. **`github-comment-card.tsx`**:
   - Added `layout?: 'inline' | 'block'` prop to `GitHubCommentCardProps`
   - Replaced the string matching logic with a simple `layout === 'inline'` check
   - Default is `'inline'` for backwards compatibility (WYSIWYG editor)

2. **`GitHubCommentsDialog.tsx`**:
   - Added `layout="block"` prop to `GitHubCommentCard`

The WYSIWYG editor (`github-comment-node.tsx`) doesn't need changes since it uses the default `layout="inline"`.

* Done! The hacky string matching has been replaced with an explicit `layout` prop:

**Summary of changes:**

1. **`github-comment-card.tsx`**:
   - Consolidated `variant` and `layout` into a single required `variant` prop
   - Three values: `'compact'` (inline chip), `'full'` (inline card with max-width), `'list'` (block card)
   - Removed the `layout` prop entirely
   - Made `variant` required (no default)

2. **`GitHubCommentsDialog.tsx`**:
   - Changed from `variant="full" layout="block"` to just `variant="list"`

3. **`github-comment-node.tsx`**:
   - Already had `variant="full"` explicitly set, no change needed
---
 .../dialogs/tasks/GitHubCommentsDialog.tsx    | 165 +++++++++---------
 frontend/src/components/ui/dialog.tsx         |   2 +-
 .../src/components/ui/github-comment-card.tsx |  16 +-
 3 files changed, 96 insertions(+), 87 deletions(-)

diff --git a/frontend/src/components/dialogs/tasks/GitHubCommentsDialog.tsx b/frontend/src/components/dialogs/tasks/GitHubCommentsDialog.tsx
index 9863b62f..567842f7 100644
--- a/frontend/src/components/dialogs/tasks/GitHubCommentsDialog.tsx
+++ b/frontend/src/components/dialogs/tasks/GitHubCommentsDialog.tsx
@@ -87,9 +87,13 @@ const GitHubCommentsDialogImpl = NiceModal.create(
     const errorMessage = isError ? getErrorMessage(error) : null;
 
     return (
-      
+      
          {
             if (e.key === 'Escape') {
               e.stopPropagation();
@@ -98,92 +102,97 @@ const GitHubCommentsDialogImpl = NiceModal.create(
             }
           }}
         >
-          
+          
             
               
               {t('tasks:githubComments.dialog.title')}
             
           
 
-          

-            {errorMessage ? (
-              
-                
-                {errorMessage}
-              
-            ) : isLoading ? (
-              

-                
-              

-            ) : comments.length === 0 ? (
-              

-                {t('tasks:githubComments.dialog.noComments')}
-              

-            ) : (
-              <>
-                

-                  
-                    {t('tasks:githubComments.dialog.selectedCount', {
-                      selected: selectedIds.size,
-                      total: comments.length,
+          

+            

+              {errorMessage ? (
+                
+                  
+                  {errorMessage}
+                
+              ) : isLoading ? (
+                

+                  
+                

+              ) : comments.length === 0 ? (
+                

+                  {t('tasks:githubComments.dialog.noComments')}
+                

+              ) : (
+                <>
+                  

+                    
+                      {t('tasks:githubComments.dialog.selectedCount', {
+                        selected: selectedIds.size,
+                        total: comments.length,
+                      })}
+                    
+                    
+                  

+                  

+                    {comments.map((comment) => {
+                      const id = getCommentId(comment);
+                      return (
+                        

+                           toggleSelection(id)}
+                            className="mt-3"
+                          />
+                           toggleSelection(id)}
+                            className="flex-1 min-w-0"
+                          />
+                        

+                      );
                     })}
-                  
-                  
-                

-                

-                  {comments.map((comment) => {
-                    const id = getCommentId(comment);
-                    return (
-                      

-                         toggleSelection(id)}
-                          className="mt-3"
-                        />
-                         toggleSelection(id)}
-                          className="block w-full max-w-none flex-1"
-                        />
-                      

-                    );
-                  })}
-                

-              
-            )}
+                  

+                
+              )}
+            

           

 
           {!errorMessage && !isLoading && comments.length > 0 && (
-            
+            
               
diff --git a/frontend/src/components/ui/dialog.tsx b/frontend/src/components/ui/dialog.tsx
index eacfc2ef..6cae8b8b 100644
--- a/frontend/src/components/ui/dialog.tsx
+++ b/frontend/src/components/ui/dialog.tsx
@@ -116,7 +116,7 @@ const Dialog = React.forwardRef<
       
 void;
   onDoubleClick?: (e: React.MouseEvent) => void;
   className?: string;
@@ -111,6 +111,7 @@ function FullCard({
   line,
   diffHunk,
   onClick,
+  variant,
   className,
 }: GitHubCommentCardProps) {
   const { t } = useTranslation('tasks');
@@ -120,7 +121,8 @@ function FullCard({
   return (
     
;
   }
-
+  // Both 'full' and 'list' use FullCard, just with different styling
   return ;
 }