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 `<pre>`) 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
This commit is contained in:
committed by
GitHub
parent
0b1af7e38a
commit
a82c7e37e7
@@ -87,9 +87,13 @@ const GitHubCommentsDialogImpl = NiceModal.create<GitHubCommentsDialogProps>(
|
|||||||
const errorMessage = isError ? getErrorMessage(error) : null;
|
const errorMessage = isError ? getErrorMessage(error) : null;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open={modal.visible} onOpenChange={handleOpenChange}>
|
<Dialog
|
||||||
|
open={modal.visible}
|
||||||
|
onOpenChange={handleOpenChange}
|
||||||
|
className="max-w-2xl p-0 overflow-hidden"
|
||||||
|
>
|
||||||
<DialogContent
|
<DialogContent
|
||||||
className="max-w-2xl p-0 flex flex-col max-h-[80vh]"
|
className="p-0"
|
||||||
onKeyDownCapture={(e) => {
|
onKeyDownCapture={(e) => {
|
||||||
if (e.key === 'Escape') {
|
if (e.key === 'Escape') {
|
||||||
e.stopPropagation();
|
e.stopPropagation();
|
||||||
@@ -98,92 +102,97 @@ const GitHubCommentsDialogImpl = NiceModal.create<GitHubCommentsDialogProps>(
|
|||||||
}
|
}
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
<DialogHeader className="px-4 py-3 border-b flex-shrink-0">
|
<DialogHeader className="px-4 py-3 border-b">
|
||||||
<DialogTitle className="flex items-center gap-2">
|
<DialogTitle className="flex items-center gap-2">
|
||||||
<MessageSquare className="h-5 w-5" />
|
<MessageSquare className="h-5 w-5" />
|
||||||
{t('tasks:githubComments.dialog.title')}
|
{t('tasks:githubComments.dialog.title')}
|
||||||
</DialogTitle>
|
</DialogTitle>
|
||||||
</DialogHeader>
|
</DialogHeader>
|
||||||
|
|
||||||
<div className="p-4 overflow-auto flex-1">
|
<div className="max-h-[70vh] flex flex-col min-h-0">
|
||||||
{errorMessage ? (
|
<div className="p-4 overflow-auto flex-1 min-h-0">
|
||||||
<Alert variant="destructive">
|
{errorMessage ? (
|
||||||
<AlertCircle className="h-4 w-4" />
|
<Alert variant="destructive">
|
||||||
<AlertDescription>{errorMessage}</AlertDescription>
|
<AlertCircle className="h-4 w-4" />
|
||||||
</Alert>
|
<AlertDescription>{errorMessage}</AlertDescription>
|
||||||
) : isLoading ? (
|
</Alert>
|
||||||
<div className="flex items-center justify-center py-8">
|
) : isLoading ? (
|
||||||
<Loader2 className="h-6 w-6 animate-spin text-muted-foreground" />
|
<div className="flex items-center justify-center py-8">
|
||||||
</div>
|
<Loader2 className="h-6 w-6 animate-spin text-muted-foreground" />
|
||||||
) : comments.length === 0 ? (
|
</div>
|
||||||
<p className="text-center text-muted-foreground py-8">
|
) : comments.length === 0 ? (
|
||||||
{t('tasks:githubComments.dialog.noComments')}
|
<p className="text-center text-muted-foreground py-8">
|
||||||
</p>
|
{t('tasks:githubComments.dialog.noComments')}
|
||||||
) : (
|
</p>
|
||||||
<>
|
) : (
|
||||||
<div className="flex items-center justify-between mb-3">
|
<>
|
||||||
<span className="text-sm text-muted-foreground">
|
<div className="flex items-center justify-between mb-3">
|
||||||
{t('tasks:githubComments.dialog.selectedCount', {
|
<span className="text-sm text-muted-foreground">
|
||||||
selected: selectedIds.size,
|
{t('tasks:githubComments.dialog.selectedCount', {
|
||||||
total: comments.length,
|
selected: selectedIds.size,
|
||||||
|
total: comments.length,
|
||||||
|
})}
|
||||||
|
</span>
|
||||||
|
<Button
|
||||||
|
variant="ghost"
|
||||||
|
size="sm"
|
||||||
|
onClick={isAllSelected ? deselectAll : selectAll}
|
||||||
|
>
|
||||||
|
{isAllSelected
|
||||||
|
? t('tasks:githubComments.dialog.deselectAll')
|
||||||
|
: t('tasks:githubComments.dialog.selectAll')}
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
<div className="space-y-3">
|
||||||
|
{comments.map((comment) => {
|
||||||
|
const id = getCommentId(comment);
|
||||||
|
return (
|
||||||
|
<div
|
||||||
|
key={id}
|
||||||
|
className="flex items-start gap-3 min-w-0"
|
||||||
|
>
|
||||||
|
<Checkbox
|
||||||
|
checked={selectedIds.has(id)}
|
||||||
|
onCheckedChange={() => toggleSelection(id)}
|
||||||
|
className="mt-3"
|
||||||
|
/>
|
||||||
|
<GitHubCommentCard
|
||||||
|
author={comment.author}
|
||||||
|
body={comment.body}
|
||||||
|
createdAt={comment.created_at}
|
||||||
|
url={comment.url}
|
||||||
|
commentType={comment.comment_type}
|
||||||
|
path={
|
||||||
|
comment.comment_type === 'review'
|
||||||
|
? comment.path
|
||||||
|
: undefined
|
||||||
|
}
|
||||||
|
line={
|
||||||
|
comment.comment_type === 'review' &&
|
||||||
|
comment.line != null
|
||||||
|
? Number(comment.line)
|
||||||
|
: undefined
|
||||||
|
}
|
||||||
|
diffHunk={
|
||||||
|
comment.comment_type === 'review'
|
||||||
|
? comment.diff_hunk
|
||||||
|
: undefined
|
||||||
|
}
|
||||||
|
variant="list"
|
||||||
|
onClick={() => toggleSelection(id)}
|
||||||
|
className="flex-1 min-w-0"
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
})}
|
})}
|
||||||
</span>
|
</div>
|
||||||
<Button
|
</>
|
||||||
variant="ghost"
|
)}
|
||||||
size="sm"
|
</div>
|
||||||
onClick={isAllSelected ? deselectAll : selectAll}
|
|
||||||
>
|
|
||||||
{isAllSelected
|
|
||||||
? t('tasks:githubComments.dialog.deselectAll')
|
|
||||||
: t('tasks:githubComments.dialog.selectAll')}
|
|
||||||
</Button>
|
|
||||||
</div>
|
|
||||||
<div className="space-y-3">
|
|
||||||
{comments.map((comment) => {
|
|
||||||
const id = getCommentId(comment);
|
|
||||||
return (
|
|
||||||
<div key={id} className="flex items-start gap-3">
|
|
||||||
<Checkbox
|
|
||||||
checked={selectedIds.has(id)}
|
|
||||||
onCheckedChange={() => toggleSelection(id)}
|
|
||||||
className="mt-3"
|
|
||||||
/>
|
|
||||||
<GitHubCommentCard
|
|
||||||
author={comment.author}
|
|
||||||
body={comment.body}
|
|
||||||
createdAt={comment.created_at}
|
|
||||||
url={comment.url}
|
|
||||||
commentType={comment.comment_type}
|
|
||||||
path={
|
|
||||||
comment.comment_type === 'review'
|
|
||||||
? comment.path
|
|
||||||
: undefined
|
|
||||||
}
|
|
||||||
line={
|
|
||||||
comment.comment_type === 'review' &&
|
|
||||||
comment.line != null
|
|
||||||
? Number(comment.line)
|
|
||||||
: undefined
|
|
||||||
}
|
|
||||||
diffHunk={
|
|
||||||
comment.comment_type === 'review'
|
|
||||||
? comment.diff_hunk
|
|
||||||
: undefined
|
|
||||||
}
|
|
||||||
variant="full"
|
|
||||||
onClick={() => toggleSelection(id)}
|
|
||||||
className="block w-full max-w-none flex-1"
|
|
||||||
/>
|
|
||||||
</div>
|
|
||||||
);
|
|
||||||
})}
|
|
||||||
</div>
|
|
||||||
</>
|
|
||||||
)}
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{!errorMessage && !isLoading && comments.length > 0 && (
|
{!errorMessage && !isLoading && comments.length > 0 && (
|
||||||
<DialogFooter className="px-4 py-3 border-t flex-shrink-0">
|
<DialogFooter className="px-4 py-3 border-t">
|
||||||
<Button variant="outline" onClick={() => handleOpenChange(false)}>
|
<Button variant="outline" onClick={() => handleOpenChange(false)}>
|
||||||
{t('common:buttons.cancel')}
|
{t('common:buttons.cancel')}
|
||||||
</Button>
|
</Button>
|
||||||
|
|||||||
@@ -116,7 +116,7 @@ const Dialog = React.forwardRef<
|
|||||||
<div
|
<div
|
||||||
ref={ref}
|
ref={ref}
|
||||||
className={cn(
|
className={cn(
|
||||||
'relative z-[9999] grid w-full max-w-lg gap-4 bg-primary p-6 shadow-lg duration-200 sm:rounded-lg my-8',
|
'relative z-[9999] flex flex-col w-full max-w-lg gap-4 bg-primary p-6 shadow-lg duration-200 sm:rounded-lg my-8',
|
||||||
className
|
className
|
||||||
)}
|
)}
|
||||||
{...props}
|
{...props}
|
||||||
|
|||||||
@@ -12,8 +12,8 @@ export interface GitHubCommentCardProps {
|
|||||||
path?: string;
|
path?: string;
|
||||||
line?: number | null;
|
line?: number | null;
|
||||||
diffHunk?: string;
|
diffHunk?: string;
|
||||||
// Display variants
|
/** Display variant: 'compact' for inline chip, 'full' for inline card, 'list' for block card */
|
||||||
variant?: 'compact' | 'full';
|
variant: 'compact' | 'full' | 'list';
|
||||||
onClick?: (e: React.MouseEvent) => void;
|
onClick?: (e: React.MouseEvent) => void;
|
||||||
onDoubleClick?: (e: React.MouseEvent) => void;
|
onDoubleClick?: (e: React.MouseEvent) => void;
|
||||||
className?: string;
|
className?: string;
|
||||||
@@ -111,6 +111,7 @@ function FullCard({
|
|||||||
line,
|
line,
|
||||||
diffHunk,
|
diffHunk,
|
||||||
onClick,
|
onClick,
|
||||||
|
variant,
|
||||||
className,
|
className,
|
||||||
}: GitHubCommentCardProps) {
|
}: GitHubCommentCardProps) {
|
||||||
const { t } = useTranslation('tasks');
|
const { t } = useTranslation('tasks');
|
||||||
@@ -120,7 +121,8 @@ function FullCard({
|
|||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
className={cn(
|
className={cn(
|
||||||
'inline-block align-bottom p-3 bg-muted/50 rounded-md border border-border cursor-pointer hover:border-muted-foreground transition-colors max-w-md',
|
'p-3 bg-muted/50 rounded-md border border-border cursor-pointer hover:border-muted-foreground transition-colors overflow-hidden',
|
||||||
|
variant === 'full' && 'inline-block align-bottom max-w-md',
|
||||||
className
|
className
|
||||||
)}
|
)}
|
||||||
onClick={onClick}
|
onClick={onClick}
|
||||||
@@ -178,14 +180,12 @@ function FullCard({
|
|||||||
/**
|
/**
|
||||||
* GitHubCommentCard - Shared presentational component for GitHub PR comments
|
* GitHubCommentCard - Shared presentational component for GitHub PR comments
|
||||||
*
|
*
|
||||||
* @param variant - 'compact' for inline WYSIWYG chip, 'full' for dialog card
|
* @param variant - 'compact' for inline chip, 'full' for inline card, 'list' for block card
|
||||||
*/
|
*/
|
||||||
export function GitHubCommentCard(props: GitHubCommentCardProps) {
|
export function GitHubCommentCard(props: GitHubCommentCardProps) {
|
||||||
const { variant = 'full' } = props;
|
if (props.variant === 'compact') {
|
||||||
|
|
||||||
if (variant === 'compact') {
|
|
||||||
return <CompactCard {...props} />;
|
return <CompactCard {...props} />;
|
||||||
}
|
}
|
||||||
|
// Both 'full' and 'list' use FullCard, just with different styling
|
||||||
return <FullCard {...props} />;
|
return <FullCard {...props} />;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user