Improve PR comment sidebar readability (#6285)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
GitHub
parent
67b1564adf
commit
75545ff09d
|
|
@ -145,6 +145,7 @@ import { localizedHostedReviewCopy } from '@/i18n/hosted-review-localized-copy'
|
|||
import { translate } from '@/i18n/i18n'
|
||||
import { groupPRComments, type PRCommentGroup } from '@/lib/pr-comment-groups'
|
||||
import { openChecksPanelHostedReviewUrl } from './checks-panel-hosted-review-click-routing'
|
||||
import { ChecksPanelUpdatedAtMetadata } from './checks-panel-updated-at-metadata'
|
||||
import {
|
||||
clearPullRequestGenerationRequiresPushBeforeCreate,
|
||||
createRunningPullRequestGenerationRecord,
|
||||
|
|
@ -3469,11 +3470,10 @@ export default function ChecksPanel(): React.JSX.Element {
|
|||
|
||||
{/* Updated at */}
|
||||
{activeReview.updatedAt && (
|
||||
<div className="text-[10px] text-muted-foreground/60">
|
||||
{reviewShortLabel}{' '}
|
||||
{translate('auto.components.right.sidebar.ChecksPanel.34464d00b9', 'updated')}
|
||||
{new Date(activeReview.updatedAt).toLocaleString()}
|
||||
</div>
|
||||
<ChecksPanelUpdatedAtMetadata
|
||||
reviewShortLabel={reviewShortLabel}
|
||||
updatedAt={activeReview.updatedAt}
|
||||
/>
|
||||
)}
|
||||
{/* Merge / Delete Workspace actions */}
|
||||
{activeReview && activeWorktree && repo && (
|
||||
|
|
|
|||
|
|
@ -0,0 +1,28 @@
|
|||
// @vitest-environment happy-dom
|
||||
|
||||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { ChecksPanelUpdatedAtMetadata } from './checks-panel-updated-at-metadata'
|
||||
|
||||
function renderMetadataText(reviewShortLabel: string): string {
|
||||
const container = document.createElement('div')
|
||||
container.innerHTML = renderToStaticMarkup(
|
||||
<ChecksPanelUpdatedAtMetadata
|
||||
reviewShortLabel={reviewShortLabel}
|
||||
updatedAt="2026-06-24T17:30:00.000Z"
|
||||
/>
|
||||
)
|
||||
return container.textContent ?? ''
|
||||
}
|
||||
|
||||
describe('ChecksPanel updated-at metadata', () => {
|
||||
it.each(['PR', 'MR'])(
|
||||
'renders a whitespace boundary between the %s updated label and timestamp',
|
||||
(reviewShortLabel) => {
|
||||
const text = renderMetadataText(reviewShortLabel)
|
||||
|
||||
expect(text).toMatch(new RegExp(`${reviewShortLabel} updated\\s+\\S`))
|
||||
expect(text).not.toMatch(new RegExp(`${reviewShortLabel} updated\\S`))
|
||||
}
|
||||
)
|
||||
})
|
||||
|
|
@ -273,6 +273,9 @@ describe('PRCommentsList', () => {
|
|||
expect(markup.indexOf('aria-label="Add comment"')).toBeLessThan(
|
||||
markup.indexOf('Existing review context')
|
||||
)
|
||||
expect(markup.indexOf('aria-label="Comment display options"')).toBeLessThan(
|
||||
markup.indexOf('aria-label="Add comment"')
|
||||
)
|
||||
expect(markup).toContain('lucide-plus')
|
||||
expect(markup).not.toContain('Add a comment...')
|
||||
expect(markup).not.toContain('Add a PR comment')
|
||||
|
|
|
|||
|
|
@ -2461,6 +2461,42 @@ export function PRCommentsList({
|
|||
)}
|
||||
</>
|
||||
)}
|
||||
{comments.length > 0 && (
|
||||
<DropdownMenu>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className="text-muted-foreground hover:text-foreground"
|
||||
aria-label={translate(
|
||||
'auto.components.right.sidebar.checks.panel.content.f5cf324efa',
|
||||
'Comment display options'
|
||||
)}
|
||||
>
|
||||
<SlidersHorizontal className="size-3" />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
<DropdownMenuContent align="end" side="bottom" sideOffset={6}>
|
||||
<DropdownMenuLabel>
|
||||
{translate(
|
||||
'auto.components.right.sidebar.checks.panel.content.5e6e5a13fa',
|
||||
'View'
|
||||
)}
|
||||
</DropdownMenuLabel>
|
||||
<DropdownMenuRadioGroup
|
||||
value={displayMode}
|
||||
onValueChange={(value) => setDisplayMode(value as PRCommentsListDisplayMode)}
|
||||
>
|
||||
{PR_COMMENT_LIST_DISPLAY_MODES.map((mode) => (
|
||||
<DropdownMenuRadioItem key={mode} value={mode}>
|
||||
{getPRCommentsListDisplayModeLabel(mode)}
|
||||
</DropdownMenuRadioItem>
|
||||
))}
|
||||
</DropdownMenuRadioGroup>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
)}
|
||||
{onAddComment && !isAddingComment && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
|
|
@ -2502,42 +2538,6 @@ export function PRCommentsList({
|
|||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)}
|
||||
{comments.length > 0 && (
|
||||
<DropdownMenu>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className="text-muted-foreground hover:text-foreground"
|
||||
aria-label={translate(
|
||||
'auto.components.right.sidebar.checks.panel.content.f5cf324efa',
|
||||
'Comment display options'
|
||||
)}
|
||||
>
|
||||
<SlidersHorizontal className="size-3" />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
<DropdownMenuContent align="end" side="bottom" sideOffset={6}>
|
||||
<DropdownMenuLabel>
|
||||
{translate(
|
||||
'auto.components.right.sidebar.checks.panel.content.5e6e5a13fa',
|
||||
'View'
|
||||
)}
|
||||
</DropdownMenuLabel>
|
||||
<DropdownMenuRadioGroup
|
||||
value={displayMode}
|
||||
onValueChange={(value) => setDisplayMode(value as PRCommentsListDisplayMode)}
|
||||
>
|
||||
{PR_COMMENT_LIST_DISPLAY_MODES.map((mode) => (
|
||||
<DropdownMenuRadioItem key={mode} value={mode}>
|
||||
{getPRCommentsListDisplayModeLabel(mode)}
|
||||
</DropdownMenuRadioItem>
|
||||
))}
|
||||
</DropdownMenuRadioGroup>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
{comments.length > 0 && (
|
||||
|
|
|
|||
|
|
@ -0,0 +1,20 @@
|
|||
import type React from 'react'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
type ChecksPanelUpdatedAtMetadataProps = {
|
||||
reviewShortLabel: string
|
||||
updatedAt: string
|
||||
}
|
||||
|
||||
export function ChecksPanelUpdatedAtMetadata({
|
||||
reviewShortLabel,
|
||||
updatedAt
|
||||
}: ChecksPanelUpdatedAtMetadataProps): React.JSX.Element {
|
||||
return (
|
||||
<div className="text-[10px] text-muted-foreground/60">
|
||||
{reviewShortLabel}{' '}
|
||||
{translate('auto.components.right.sidebar.ChecksPanel.34464d00b9', 'updated')}{' '}
|
||||
{new Date(updatedAt).toLocaleString()}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
|
@ -15,15 +15,28 @@ describe('pr-comment-presentation', () => {
|
|||
it('returns card layout tokens for cards and focus variants', () => {
|
||||
const cards = getPRCommentPresentationClasses('cards')
|
||||
expect(cards.useCardLayout).toBe(true)
|
||||
expect(cards.commentBody).toContain('text-[13px]')
|
||||
expect(cards.commentBody).toContain('text-xs')
|
||||
expect(cards.commentBody).toContain('leading-5')
|
||||
expect(cards.commentBody).toContain('text-foreground')
|
||||
expect(cards.group).toContain('bg-secondary')
|
||||
expect(cards.group).toContain('shadow-xs')
|
||||
expect(cards.avatar).toContain('border-border')
|
||||
expect(cards.avatar).toContain('bg-background')
|
||||
|
||||
expect(getPRCommentPresentationClasses('focus').useCardLayout).toBe(true)
|
||||
expect(getPRCommentPresentationClasses('focus').commentBody).toContain('text-[14px]')
|
||||
const focus = getPRCommentPresentationClasses('focus')
|
||||
expect(focus.useCardLayout).toBe(true)
|
||||
expect(focus.commentBody).toContain('text-xs')
|
||||
expect(focus.commentBody).toContain('leading-5')
|
||||
expect(focus.commentBodyReply).toContain('text-xs')
|
||||
expect(focus.commentBodyReply).toContain('leading-5')
|
||||
expect(focus.author).toContain('text-[13px]')
|
||||
expect(focus.list).toContain('gap-2')
|
||||
expect(focus.commentBody).toContain('px-4 py-2.5')
|
||||
expect(focus.commentBodyReply).toContain('px-4 py-2.5')
|
||||
expect(focus.commentHeader).toContain('px-3 py-2')
|
||||
expect(focus.commentHeaderReply).toContain('px-3 py-2')
|
||||
expect(focus.commentHeaderMeta).toContain('pl-7')
|
||||
expect(focus.commentHeaderMetaWithSelection).toContain('pl-[3.25rem]')
|
||||
})
|
||||
|
||||
it('preserves the legacy flat layout tokens', () => {
|
||||
|
|
|
|||
|
|
@ -96,6 +96,15 @@ const COMMENT_AVATAR =
|
|||
const RESOLVED_SECTION_LABEL =
|
||||
'text-[11px] font-semibold uppercase tracking-wider text-muted-foreground'
|
||||
|
||||
// Why: markdown bodies need the secondary sidebar scale; 13px relaxed copy reads oversized in narrow comment cards.
|
||||
const CARD_COMMENT_BODY_SIZE = 'text-xs leading-5'
|
||||
const CARD_COMMENT_AUTHOR_SIZE = 'text-[13px]'
|
||||
const CARD_COMMENT_LIST_GAP = 'gap-2'
|
||||
const CARD_COMMENT_BODY_PADDING = 'px-4 py-2.5'
|
||||
const CARD_COMMENT_HEADER_PADDING = 'px-3 py-2'
|
||||
const CARD_COMMENT_META_INDENT = 'pl-7'
|
||||
const CARD_COMMENT_META_SELECTION_INDENT = 'pl-[3.25rem]'
|
||||
|
||||
const RESOLVED_SECTION_TRIGGER = cn(
|
||||
RESOLVED_SECTION_LABEL,
|
||||
'rounded-none border-0 bg-transparent px-3 py-2 shadow-none hover:bg-accent/40 hover:text-foreground hover:no-underline'
|
||||
|
|
@ -172,29 +181,21 @@ export function getPRCommentPresentationClasses(
|
|||
}
|
||||
}
|
||||
|
||||
const isFocus = variant === 'focus'
|
||||
const bodySize = isFocus ? 'text-[14px] leading-relaxed' : 'text-[13px] leading-relaxed'
|
||||
const authorSize = isFocus ? 'text-[14px]' : 'text-[13px]'
|
||||
const listGap = isFocus ? 'gap-3' : 'gap-2'
|
||||
const bodyPadding = isFocus ? 'px-4 py-3' : 'px-4 py-2.5'
|
||||
const headerPadding = isFocus ? 'px-3.5 py-2.5' : 'px-3 py-2'
|
||||
const metaIndent = isFocus ? 'pl-8' : 'pl-7'
|
||||
|
||||
return {
|
||||
variant,
|
||||
useCardLayout: true,
|
||||
list: `flex flex-col ${listGap} px-3 py-2`,
|
||||
list: `flex flex-col ${CARD_COMMENT_LIST_GAP} px-3 py-2`,
|
||||
group: COMMENT_CARD_SURFACE,
|
||||
groupStandalone: '',
|
||||
groupThread: '',
|
||||
commentRow: 'group/comment',
|
||||
commentRowReply: `border-t ${COMMENT_CARD_DIVIDER} bg-muted/25 dark:bg-muted/10`,
|
||||
commentHeader: `flex flex-col gap-1 border-b ${COMMENT_CARD_DIVIDER} ${headerPadding}`,
|
||||
commentHeaderReply: `flex min-w-0 items-center gap-2 ${headerPadding}`,
|
||||
commentBody: `${bodyPadding} ${bodySize} text-foreground`,
|
||||
commentBodyReply: `${bodyPadding} ${bodySize} text-foreground`,
|
||||
commentHeader: `flex flex-col gap-1 border-b ${COMMENT_CARD_DIVIDER} ${CARD_COMMENT_HEADER_PADDING}`,
|
||||
commentHeaderReply: `flex min-w-0 items-center gap-2 ${CARD_COMMENT_HEADER_PADDING}`,
|
||||
commentBody: `${CARD_COMMENT_BODY_PADDING} ${CARD_COMMENT_BODY_SIZE} text-foreground`,
|
||||
commentBodyReply: `${CARD_COMMENT_BODY_PADDING} ${CARD_COMMENT_BODY_SIZE} text-foreground`,
|
||||
commentBodyMarkdown: MARKDOWN_BASE,
|
||||
author: `min-w-0 flex-1 truncate ${authorSize} font-semibold text-foreground`,
|
||||
author: `min-w-0 flex-1 truncate ${CARD_COMMENT_AUTHOR_SIZE} font-semibold text-foreground`,
|
||||
authorResolved: 'text-muted-foreground',
|
||||
avatar: `size-5 ${COMMENT_AVATAR}`,
|
||||
avatarReply: `size-4 ${COMMENT_AVATAR}`,
|
||||
|
|
@ -224,12 +225,12 @@ export function getPRCommentPresentationClasses(
|
|||
'shrink-0 rounded border border-ring/40 bg-accent px-1.5 py-0.5 text-[10px] font-semibold uppercase tracking-wide text-foreground',
|
||||
commentHeaderPrimary: 'flex min-w-0 items-center gap-2',
|
||||
commentHeaderMeta: cn(
|
||||
metaIndent,
|
||||
CARD_COMMENT_META_INDENT,
|
||||
'flex min-w-0 flex-wrap items-center gap-x-2 gap-y-1 text-[11px] text-muted-foreground'
|
||||
),
|
||||
// Why: checkbox (16px) + gap-2 sits before the avatar row the meta row already indents past.
|
||||
commentHeaderMetaWithSelection: cn(
|
||||
isFocus ? 'pl-[3.5rem]' : 'pl-[3.25rem]',
|
||||
CARD_COMMENT_META_SELECTION_INDENT,
|
||||
'flex min-w-0 flex-wrap items-center gap-x-2 gap-y-1 text-[11px] text-muted-foreground'
|
||||
),
|
||||
// Why: open state is conveyed by the status badge; a green card rail reads noisy in the sidebar.
|
||||
|
|
|
|||
Loading…
Reference in New Issue