From c20f73dbec492509d4b25a90c3b6bcda926f3428 Mon Sep 17 00:00:00 2001 From: Ryota Murakami Date: Sun, 18 Oct 2026 22:48:48 +0900 Subject: [PATCH 1/3] feat: close leftover 2026-10-12 design gaps Settings gets the signed-in sidebar, a closed note returns focus to the card, transitions name the properties that change, and the picker uses a Radix visibility list and a lucide star. --- e2e/logged-in/add-repository-combobox.spec.ts | 97 +++++++++++++ e2e/logged-in/note-modal.spec.ts | 21 +++ e2e/logged-in/settings.spec.ts | 1 + e2e/logged-in/sidebar-narrow-viewport.spec.ts | 5 +- src/app/(landing)/KanbanPreview.tsx | 4 +- src/app/boards/new/CreateBoardForm.tsx | 3 +- src/app/settings/SettingsLayoutClient.tsx | 37 +++++ src/app/settings/layout.tsx | 20 +-- .../Board/AddRepositoryCombobox.tsx | 44 ++++-- src/components/Board/ColumnInsertZone.tsx | 4 +- src/components/Board/CommentDisplay.tsx | 4 +- src/components/Board/MoveHereDialog.tsx | 3 +- src/components/Board/NewRowDropZone.tsx | 2 +- src/components/Board/RepoCard.tsx | 2 +- src/components/Board/SortableColumn.tsx | 2 +- src/components/Board/StatusColumn.tsx | 2 + src/components/Boards/BoardCard.tsx | 4 +- src/components/Boards/BoardSettingsDialog.tsx | 23 +-- src/components/Boards/DeleteBoardDialog.tsx | 3 +- src/components/Boards/RenameBoardDialog.tsx | 12 +- src/components/Boards/SortableBoardCard.tsx | 2 +- .../Modals/CreateDestinationBoardForm.tsx | 11 +- .../Modals/CreateLinkTypeDialog.tsx | 19 ++- src/components/Modals/DeleteAccountDialog.tsx | 3 +- .../Modals/MoveToAnotherBoardDialog.tsx | 20 +-- src/components/Modals/NoteModal.stories.tsx | 134 +++++++++++++++++- src/components/Modals/NoteModal.tsx | 6 +- src/components/Modals/StatusListDialog.tsx | 5 +- src/components/Sidebar/Sidebar.tsx | 2 +- src/components/ui/editable-url-item.tsx | 11 +- .../ui/font-color-toolbar-button.tsx | 2 +- src/components/ui/form-error.tsx | 47 ++++++ src/hooks/board/useCardDialogFocusReturn.ts | 4 +- src/lib/utils/pending-button-label.ts | 23 +++ .../AddRepositoryCombobox.placements.test.tsx | 33 ++++- .../components/Board/StatusColumn.test.tsx | 27 ++++ .../unit/components/Sidebar/Sidebar.test.tsx | 1 + .../unit/components/ui/form-error.test.tsx | 52 +++++++ .../copy/transition-all-call-sites.test.ts | 76 ++++++++++ .../lib/utils/pending-button-label.test.ts | 37 +++++ 40 files changed, 698 insertions(+), 110 deletions(-) create mode 100644 src/app/settings/SettingsLayoutClient.tsx create mode 100644 src/components/ui/form-error.tsx create mode 100644 src/lib/utils/pending-button-label.ts create mode 100644 src/tests/unit/components/ui/form-error.test.tsx create mode 100644 src/tests/unit/copy/transition-all-call-sites.test.ts create mode 100644 src/tests/unit/lib/utils/pending-button-label.test.ts diff --git a/e2e/logged-in/add-repository-combobox.spec.ts b/e2e/logged-in/add-repository-combobox.spec.ts index e0556697..d01eb7c0 100644 --- a/e2e/logged-in/add-repository-combobox.spec.ts +++ b/e2e/logged-in/add-repository-combobox.spec.ts @@ -1399,6 +1399,103 @@ test.describe('AddRepositoryCombobox - Escape', () => { // Assert await expect(searchBox).toBeHidden() }) + + // Value: protects=Escape in the open Visibility list closes that list only; + // fails_when=the panel takes the Escape of the list and the picker is gone; + // why_new=Visibility is now a Radix select like Organization; seam=none + test('Escape in the open Visibility list closes the list, and the picker stays', async ({ + page, + }) => { + // Arrange + await page.goto(BOARD_URL) + await expect(page.getByText('Pending')).toBeVisible({ timeout: 15000 }) + await page.getByRole('button', { name: /add repositories/i }).click() + const searchBox = page.getByRole('textbox', { + name: 'Search repositories', + }) + await expect(searchBox).toBeVisible() + const visibilityFilter = page.getByRole('combobox', { + name: 'Visibility filter', + }) + await visibilityFilter.click() + const publicOption = page.getByRole('option', { + name: 'Public', + exact: true, + }) + await expect(publicOption).toBeVisible() + + // Act + await page.keyboard.press('Escape') + + // Assert + await expect(publicOption).toBeHidden() + await expect(searchBox).toBeVisible() + }) +}) + +test.describe('AddRepositoryCombobox - Visibility and stars', () => { + test.use({ storageState: 'e2e/.auth/user.json' }) + + const BOARD_URL = `/board/${BOARD_IDS.testBoard}` + + // Value: protects=Public hides the private fixture repo and Private shows it; + // fails_when=the filter is ignored or a native select cannot be driven; + // why_new=Visibility is now a Radix select; seam=none + test('Public hides the private fixture repository and Private shows it', async ({ + page, + }) => { + // Arrange + await page.goto(BOARD_URL) + await expect(page.getByText('Pending')).toBeVisible({ timeout: 15000 }) + await page.getByRole('button', { name: /add repositories/i }).click() + const visibilityFilter = page.getByRole('combobox', { + name: 'Visibility filter', + }) + const privateRepo = page.getByRole('option', { + name: /testuser\/private-project/, + }) + const publicRepo = page.getByRole('option', { name: /laststance\/nsx/ }) + await expect(privateRepo).toBeVisible({ timeout: 10000 }) + await expect(publicRepo).toBeVisible() + + // Act + await visibilityFilter.click() + await page.getByRole('option', { name: 'Public', exact: true }).click() + + // Assert + await expect(privateRepo).toBeHidden() + await expect(publicRepo).toBeVisible() + + // Act + await visibilityFilter.click() + await page.getByRole('option', { name: 'Private', exact: true }).click() + + // Assert + await expect(privateRepo).toBeVisible() + await expect(publicRepo).toBeHidden() + }) + + // Value: protects=an available fixture row shows a lucide star, not ⭐; + // fails_when=the emoji comes back; why_new=the leftover used ⭐; seam=none + test('shows a lucide star next to the fixture star count', async ({ + page, + }) => { + // Arrange: laststance/nsx is not on the board and has 85 stars + await page.goto(BOARD_URL) + await expect(page.getByText('Pending')).toBeVisible({ timeout: 15000 }) + await page.getByRole('button', { name: /add repositories/i }).click() + const nsx = page.getByRole('option', { name: /laststance\/nsx/ }) + await expect(nsx).toBeVisible({ timeout: 10000 }) + + // Act + const starCount = nsx.getByTestId('repo-star-count') + + // Assert + await expect(starCount).toBeVisible() + await expect(starCount).toHaveTextContent('85') + await expect(starCount.locator('svg')).toBeVisible() + await expect(starCount).not.toHaveTextContent('⭐') + }) }) /** diff --git a/e2e/logged-in/note-modal.spec.ts b/e2e/logged-in/note-modal.spec.ts index d35928c2..68734159 100644 --- a/e2e/logged-in/note-modal.spec.ts +++ b/e2e/logged-in/note-modal.spec.ts @@ -58,6 +58,27 @@ test.describe('NoteModal (Authenticated)', () => { await expect(title).toBeVisible() }) + // Value: protects=Escape on the note hands the keyboard back to the card; + // fails_when=the dialog closes onto the page itself; why_new=NoteModal now + // uses useCardDialogFocusReturn; seam=none + test('Escape on the note returns focus to the fixture card', async ({ + page, + }) => { + // Arrange + const card = page.locator(`[data-repo-card-id="${CARD_IDS.card1}"]`) + await expect(card).toBeVisible({ timeout: 10000 }) + await card.getByRole('button', { name: 'Open note' }).click() + const dialog = page.getByTestId('note-modal') + await expect(dialog).toBeVisible({ timeout: 5000 }) + + // Act + await page.keyboard.press('Escape') + + // Assert + await expect(dialog).toBeHidden() + await expect(card).toBeFocused() + }) + test('should display Plate Editor in the modal', async ({ page }) => { // Open NoteModal via Note button const card = page.locator('[data-testid^="repo-card-"]').first() diff --git a/e2e/logged-in/settings.spec.ts b/e2e/logged-in/settings.spec.ts index 3ad86039..daa419fc 100644 --- a/e2e/logged-in/settings.spec.ts +++ b/e2e/logged-in/settings.spec.ts @@ -20,6 +20,7 @@ test.describe('Settings Page (Authenticated)', () => { // Should have settings heading const heading = page.getByRole('heading', { name: /settings/i }) await expect(heading).toBeVisible() + await expect(page.getByTestId('sidebar-place')).toBeVisible() }) test('should display display settings', async ({ page }) => { diff --git a/e2e/logged-in/sidebar-narrow-viewport.spec.ts b/e2e/logged-in/sidebar-narrow-viewport.spec.ts index 46f76627..3a876f74 100644 --- a/e2e/logged-in/sidebar-narrow-viewport.spec.ts +++ b/e2e/logged-in/sidebar-narrow-viewport.spec.ts @@ -103,15 +103,16 @@ test.describe('Sidebar below 768px', () => { expect(pageWidthPx).toBe(375) }) - // Value: protects=a key typed while the sidebar lies over a board or over the account page cannot reach that page, so Z does not undo a drag behind the sidebar; fails_when=one of these layouts wraps the sidebar in an element of its own, which leaves it no sibling to set aside; why_new=the tests above open the sidebar on the boards list only, and the unit tests render a page of their own beside it; seam=none; no_claim=that Z does nothing on an inert page, which useKanbanUndo.test.tsx checks + // Value: protects=a key typed while the sidebar lies over a board, the account page, or settings cannot reach that page, so Z does not undo a drag behind the sidebar; fails_when=one of these layouts wraps the sidebar in an element of its own, which leaves it no sibling to set aside; why_new=the tests above open the sidebar on the boards list only, and the unit tests render a page of their own beside it; seam=none; no_claim=that Z does nothing on an inert page, which useKanbanUndo.test.tsx checks for (const { pageName, url } of [ { pageName: 'a board', url: `/board/${BOARD_IDS.testBoard}` }, { pageName: 'the account page', url: '/account' }, + { pageName: 'the settings page', url: '/settings' }, ]) { test(`opened over ${pageName}, it sets the page aside as on the boards list`, async ({ page, }) => { - // Arrange: one of the two other layouts of the app that hold a sidebar + // Arrange: one of the other layouts of the app that hold a sidebar await page.goto(url) const content = page.locator('main') const expandButton = page.getByRole('button', { name: 'Expand sidebar' }) diff --git a/src/app/(landing)/KanbanPreview.tsx b/src/app/(landing)/KanbanPreview.tsx index d640c3ef..9eeed53b 100644 --- a/src/app/(landing)/KanbanPreview.tsx +++ b/src/app/(landing)/KanbanPreview.tsx @@ -410,7 +410,7 @@ export const KanbanPreview = memo(function KanbanPreview() { } }} className={cn( - 'rounded-lg border p-3 transition-all duration-200', + 'rounded-lg border p-3 transition-colors duration-200', // Empty placeholder column styling column.id.startsWith('empty-') ? 'border-border/30 border-dashed bg-transparent' @@ -471,7 +471,7 @@ export const KanbanPreview = memo(function KanbanPreview() { }} onDragEnd={handleCardDragEnd} className={cn( - 'rounded-lg border p-3 transition-all duration-200', + 'rounded-lg border p-3 transition-colors duration-200', 'cursor-grab hover:scale-[1.02] hover:shadow-md active:cursor-grabbing', card.color, draggedCard?.id === card.id && 'opacity-50', diff --git a/src/app/boards/new/CreateBoardForm.tsx b/src/app/boards/new/CreateBoardForm.tsx index b758a970..4815d25c 100644 --- a/src/app/boards/new/CreateBoardForm.tsx +++ b/src/app/boards/new/CreateBoardForm.tsx @@ -40,6 +40,7 @@ import { OUTCOME_UNKNOWN_TEXT_CLASS, } from '@/lib/constants/outcome-unknown' import { areServerRenderedPropsEqual } from '@/lib/utils/are-server-rendered-props-equal' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' import { reloadPage } from '@/lib/utils/reload-page' import { requestBoardCreation } from '@/lib/utils/request-board-creation' import { BOARD_NAME_MAX_LENGTH, boardNameSchema } from '@/lib/validations/board' @@ -260,7 +261,7 @@ export const CreateBoardForm = memo(function CreateBoardForm() { ) : ( )} diff --git a/src/app/settings/SettingsLayoutClient.tsx b/src/app/settings/SettingsLayoutClient.tsx new file mode 100644 index 00000000..dbeab797 --- /dev/null +++ b/src/app/settings/SettingsLayoutClient.tsx @@ -0,0 +1,37 @@ +'use client' + +import React, { memo } from 'react' + +import { Sidebar } from '@/components/Sidebar' +import { ThemeApplicator } from '@/components/ThemeApplicator' +import { areServerRenderedPropsEqual } from '@/lib/utils/are-server-rendered-props-equal' + +interface SettingsLayoutClientProps { + children: React.ReactNode + userName?: string + userAvatar?: string +} + +/** + * Settings layout chrome: sidebar beside the page. + * + * Copied from {@link AccountLayoutClient} so `/settings` is a sibling of + * the sidebar. A sidebar that lies over the page can then set `inert` on + * `main`. {@link areServerRenderedPropsEqual} keeps the client tree when + * the server sends the layout again. + */ +export const SettingsLayoutClient = memo(function SettingsLayoutClient({ + children, + userName, + userAvatar, +}: SettingsLayoutClientProps) { + return ( + <> + +
+ +
{children}
+
+ + ) +}, areServerRenderedPropsEqual) diff --git a/src/app/settings/layout.tsx b/src/app/settings/layout.tsx index 1d798aed..d7481eb2 100644 --- a/src/app/settings/layout.tsx +++ b/src/app/settings/layout.tsx @@ -1,27 +1,27 @@ /** * Settings Layout * - * Layout for /settings route - * - Theme application for authenticated users - * - Auth gate via JWT claims (~5ms WebCrypto verify) instead of the ~50ms - * `auth.getUser()` round-trip. Claims body is unused here; the call exists - * for its redirect side effect when the JWT is missing or expired. + * Same sidebar chrome as `/account` and `/boards`. The sidebar and the + * page are siblings so a sidebar that lies over the page can set `inert` + * on `main`. */ -import { ThemeApplicator } from '@/components/ThemeApplicator' import { requireClaims } from '@/lib/auth/require-claims' +import { getClaimsDisplayInfo } from '@/lib/utils/get-claims-display-info' + +import { SettingsLayoutClient } from './SettingsLayoutClient' export default async function SettingsLayout({ children, }: { children: React.ReactNode }) { - await requireClaims() + const { claims } = await requireClaims() + const { userName, userAvatar } = getClaimsDisplayInfo(claims) return ( - <> - + {children} - + ) } diff --git a/src/components/Board/AddRepositoryCombobox.tsx b/src/components/Board/AddRepositoryCombobox.tsx index 7533eaa5..0ef2ea0f 100644 --- a/src/components/Board/AddRepositoryCombobox.tsx +++ b/src/components/Board/AddRepositoryCombobox.tsx @@ -1,6 +1,7 @@ 'use client' import { useVirtualizer } from '@tanstack/react-virtual' +import { Star } from 'lucide-react' import { useState, useRef, @@ -650,20 +651,25 @@ export const AddRepositoryCombobox = memo(function AddRepositoryCombobox({ )} {/* Visibility filter */} - + + + + + All + Public + Private + + {/* Selected repositories badges */} @@ -816,7 +822,13 @@ export const AddRepositoryCombobox = memo(function AddRepositoryCombobox({

)}
- ⭐ {repo.stargazers_count} + + {repo.language && ( {repo.language} @@ -869,7 +881,13 @@ export const AddRepositoryCombobox = memo(function AddRepositoryCombobox({

)}
- ⭐ {repo.stargazers_count} + + {repo.language && ( {repo.language} diff --git a/src/components/Board/ColumnInsertZone.tsx b/src/components/Board/ColumnInsertZone.tsx index 406843dc..cd867758 100644 --- a/src/components/Board/ColumnInsertZone.tsx +++ b/src/components/Board/ColumnInsertZone.tsx @@ -55,7 +55,7 @@ export const ColumnInsertZone = memo(
(
diff --git a/src/components/Board/CommentDisplay.tsx b/src/components/Board/CommentDisplay.tsx index 793d3058..e88cf568 100644 --- a/src/components/Board/CommentDisplay.tsx +++ b/src/components/Board/CommentDisplay.tsx @@ -174,7 +174,7 @@ export const CommentDisplay = memo( className={cn( 'border-muted-foreground/30 rounded-md border-2 border-dashed p-3', 'hover:border-muted-foreground/50 hover:bg-muted/10', - 'cursor-pointer transition-all duration-200', + 'cursor-pointer transition-colors duration-200', 'focus:ring-ring focus:ring-2 focus:ring-offset-2 focus:outline-none', className, )} @@ -205,7 +205,7 @@ export const CommentDisplay = memo( data-testid="comment-display" className={cn( // Base styles - full rounded card with background fill - 'cursor-pointer rounded-md p-3 transition-all duration-200', + 'cursor-pointer rounded-md p-3 transition-colors duration-200', // Combined card color (bg + border from COMMENT_CARD_COLORS) cardColorClass, // Focus styles diff --git a/src/components/Board/MoveHereDialog.tsx b/src/components/Board/MoveHereDialog.tsx index 932c0731..af00b283 100644 --- a/src/components/Board/MoveHereDialog.tsx +++ b/src/components/Board/MoveHereDialog.tsx @@ -37,6 +37,7 @@ import { import { useAppDispatch, useAppSelector } from '@/lib/redux/store' import type { BoardId, StatusListId } from '@/lib/types/brands' import { toMoveFailure } from '@/lib/utils/move-failure' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' import { reloadPage } from '@/lib/utils/reload-page' import { settleWithin } from '@/lib/utils/settle-within' @@ -247,7 +248,7 @@ export const MoveHereDialog = memo(function MoveHereDialog({ (failure !== null && !failure.isOutcomeKnown) } > - {isBusy ? 'Moving…' : 'Move Here'} + {pendingButtonLabel(isBusy, 'Move Here', 'Moving…')} diff --git a/src/components/Board/NewRowDropZone.tsx b/src/components/Board/NewRowDropZone.tsx index a7f9b3aa..a8c7c7ec 100644 --- a/src/components/Board/NewRowDropZone.tsx +++ b/src/components/Board/NewRowDropZone.tsx @@ -51,7 +51,7 @@ export const NewRowDropZone = memo( 'h-20 rounded-xl border-2 border-dashed', 'flex items-center justify-center', 'text-muted-foreground text-sm font-medium', - 'transition-all duration-200', + 'transition-[color,background-color,border-color,transform] duration-200', isOver ? 'border-primary bg-primary/10 text-link scale-[1.02]' : 'border-border/50 bg-background/30', diff --git a/src/components/Board/RepoCard.tsx b/src/components/Board/RepoCard.tsx index 8f095d90..8a2915cc 100644 --- a/src/components/Board/RepoCard.tsx +++ b/src/components/Board/RepoCard.tsx @@ -224,7 +224,7 @@ const RepoCardView = memo(function RepoCardView({ className={sortable ? 'cursor-grab active:cursor-grabbing' : undefined} > (
{/* StatusColumn renders the column content with draggable header */} diff --git a/src/components/Board/StatusColumn.tsx b/src/components/Board/StatusColumn.tsx index 08a82074..2828429b 100644 --- a/src/components/Board/StatusColumn.tsx +++ b/src/components/Board/StatusColumn.tsx @@ -11,6 +11,7 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, + DropdownMenuSeparator, DropdownMenuTrigger, } from '@/components/ui/dropdown-menu' import type { CommentData } from '@/lib/actions/project-info' @@ -159,6 +160,7 @@ export const StatusColumn = memo( Edit Column )} + {onEditStatus && onDeleteStatus && } {onDeleteStatus && ( onDeleteStatus(status.id)} diff --git a/src/components/Boards/BoardCard.tsx b/src/components/Boards/BoardCard.tsx index 097a209d..a8a00fa5 100644 --- a/src/components/Boards/BoardCard.tsx +++ b/src/components/Boards/BoardCard.tsx @@ -138,7 +138,7 @@ export const BoardCard = memo(function BoardCard({ <>
{/* Board link - covers the card but not the menu */} {hasRenameError ? ( - + message={renameState.errors?.name?.[0]} + /> ) : ( )} @@ -526,7 +525,7 @@ export const BoardSettingsDialog = memo(function BoardSettingsDialog({ disabled={isRenamePending || name.trim() === ''} aria-label={`Rename board to ${name}`} > - {isRenamePending ? 'Renaming…' : 'Rename'} + {pendingButtonLabel(isRenamePending, 'Rename', 'Renaming…')} @@ -823,7 +822,11 @@ export const BoardSettingsDialog = memo(function BoardSettingsDialog({ disabled={isDeletePending} aria-label={`Delete board ${boardName}`} > - {isDeletePending ? 'Deleting…' : 'Delete Board'} + {pendingButtonLabel( + isDeletePending, + 'Delete Board', + 'Deleting…', + )} diff --git a/src/components/Boards/DeleteBoardDialog.tsx b/src/components/Boards/DeleteBoardDialog.tsx index 616545b2..f86473d7 100644 --- a/src/components/Boards/DeleteBoardDialog.tsx +++ b/src/components/Boards/DeleteBoardDialog.tsx @@ -21,6 +21,7 @@ import { } from '@/components/ui/alert-dialog' import { Button } from '@/components/ui/button' import { deleteBoardAction, type DeleteBoardState } from '@/lib/actions/board' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' interface DeleteBoardDialogProps { /** Whether the dialog is open */ @@ -101,7 +102,7 @@ export const DeleteBoardDialog = memo(function DeleteBoardDialog({ disabled={isPending} aria-label={`Delete board ${boardName}`} > - {isPending ? 'Deleting…' : 'Delete Board'} + {pendingButtonLabel(isPending, 'Delete Board', 'Deleting…')} diff --git a/src/components/Boards/RenameBoardDialog.tsx b/src/components/Boards/RenameBoardDialog.tsx index cf0c7df0..8d470d0d 100644 --- a/src/components/Boards/RenameBoardDialog.tsx +++ b/src/components/Boards/RenameBoardDialog.tsx @@ -19,9 +19,11 @@ import { DialogHeader, DialogTitle, } from '@/components/ui/dialog' +import { FormError } from '@/components/ui/form-error' import { Input } from '@/components/ui/input' import { renameBoardAction, type RenameBoardState } from '@/lib/actions/board' import { NEAR_LIMIT_COUNTER_TEXT_CLASS } from '@/lib/constants/character-counter' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' import { BOARD_NAME_MAX_LENGTH } from '@/lib/validations/board' interface RenameBoardDialogProps { @@ -127,13 +129,7 @@ export const RenameBoardDialog = memo(function RenameBoardDialog({
{hasError ? ( - + ) : ( )} @@ -157,7 +153,7 @@ export const RenameBoardDialog = memo(function RenameBoardDialog({ disabled={isPending || name.trim() === ''} aria-label={`Rename board to ${name}`} > - {isPending ? 'Renaming…' : 'Rename'} + {pendingButtonLabel(isPending, 'Rename', 'Renaming…')} diff --git a/src/components/Boards/SortableBoardCard.tsx b/src/components/Boards/SortableBoardCard.tsx index d5ed6370..ef216e4c 100644 --- a/src/components/Boards/SortableBoardCard.tsx +++ b/src/components/Boards/SortableBoardCard.tsx @@ -67,7 +67,7 @@ export const SortableBoardCard = memo(function SortableBoardCard({
{/* Drag handle — visible on hover, positioned at left edge */} diff --git a/src/components/Modals/CreateDestinationBoardForm.tsx b/src/components/Modals/CreateDestinationBoardForm.tsx index 0be67233..399c3819 100644 --- a/src/components/Modals/CreateDestinationBoardForm.tsx +++ b/src/components/Modals/CreateDestinationBoardForm.tsx @@ -18,6 +18,7 @@ import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { Label } from '@/components/ui/label' import { OUTCOME_UNKNOWN_TEXT_CLASS } from '@/lib/constants/outcome-unknown' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' import { reloadPage } from '@/lib/utils/reload-page' import type { BoardCreationAttempt } from '@/lib/utils/request-board-creation' import { BOARD_NAME_MAX_LENGTH, boardNameSchema } from '@/lib/validations/board' @@ -197,13 +198,9 @@ export const CreateDestinationBoardForm = memo( {recovery === 'create' ? ( ) : (
@@ -208,7 +205,7 @@ export const CreateLinkTypeDialog = memo(function CreateLinkTypeDialog({ disabled={isPending || !label.trim()} data-testid="submit-create-link-type" > - {isPending ? 'Creating…' : 'Create'} + {pendingButtonLabel(isPending, 'Create', 'Creating…')} diff --git a/src/components/Modals/DeleteAccountDialog.tsx b/src/components/Modals/DeleteAccountDialog.tsx index 592df604..771ca31d 100644 --- a/src/components/Modals/DeleteAccountDialog.tsx +++ b/src/components/Modals/DeleteAccountDialog.tsx @@ -27,6 +27,7 @@ import { Label } from '@/components/ui/label' import { deleteAccount } from '@/lib/actions/auth' import { forgetAccountOnThisBrowser } from '@/lib/redux/forget-account-on-this-browser' import { store } from '@/lib/redux/store' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' /** Confirmation text required to enable delete button */ const CONFIRMATION_TEXT = 'DELETE' @@ -136,7 +137,7 @@ export const DeleteAccountDialog = memo(function DeleteAccountDialog({ disabled={!isConfirmed || isPending} data-testid="confirm-delete-button" > - {isPending ? 'Deleting…' : 'Delete Account'} + {pendingButtonLabel(isPending, 'Delete Account', 'Deleting…')} diff --git a/src/components/Modals/MoveToAnotherBoardDialog.tsx b/src/components/Modals/MoveToAnotherBoardDialog.tsx index 24ddfe07..4c25b1b6 100644 --- a/src/components/Modals/MoveToAnotherBoardDialog.tsx +++ b/src/components/Modals/MoveToAnotherBoardDialog.tsx @@ -48,6 +48,7 @@ import { DialogHeader, DialogTitle, } from '@/components/ui/dialog' +import { FormError } from '@/components/ui/form-error' import { Label } from '@/components/ui/label' import { Select, @@ -76,6 +77,7 @@ import { import { useAppDispatch, useAppSelector } from '@/lib/redux/store' import type { RepoCardId } from '@/lib/types/brands' import { toMoveFailure } from '@/lib/utils/move-failure' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' import { reloadPage } from '@/lib/utils/reload-page' import { requestBoardCreation, @@ -392,13 +394,7 @@ export const MoveToAnotherBoardDialog = memo(function MoveToAnotherBoardDialog({ )) .with({ kind: 'load-failed' }, ({ message }) => (
- + diff --git a/src/components/Modals/NoteModal.stories.tsx b/src/components/Modals/NoteModal.stories.tsx index 7ad0ee17..5f9e80b0 100644 --- a/src/components/Modals/NoteModal.stories.tsx +++ b/src/components/Modals/NoteModal.stories.tsx @@ -9,7 +9,7 @@ */ import type { Meta, StoryObj } from '@storybook/nextjs-vite' -import { memo, useEffect } from 'react' +import { memo, useEffect, useState, type ComponentProps } from 'react' import { expect, userEvent, waitFor, fn } from 'storybook/test' import { updateDraftNote } from '@/lib/redux/slices/draftSlice' @@ -261,3 +261,135 @@ export const CharacterCountDisplays: Story = { }) }, } + +/** + * Card the note was opened from, plus a note that can actually close. + * Story args keep `isOpen` true; without this wrapper Cancel, the overlay + * and Save never unmount the dialog, so focus never returns. + */ +const ClosableNoteOnPage = memo(function ClosableNoteOnPage( + props: ComponentProps, +) { + const [open, setOpen] = useState(props.isOpen) + return ( + <> +
+ Fixture card +
+ { + setOpen(false) + props.onClose() + }} + /> + + ) +}) + +/** + * Cancel hands the keyboard back to the card, not the page. + */ +export const CancelReturnsFocusToTheCard: Story = { + args: { + isOpen: true, + onClose: fn(), + onSave: fn(), + cardId: toRepoCardId('card-test'), + initialNote: 'Test note', + initialLinks: [], + cardTitle: 'test/repo', + }, + render: (args) => , + play: async () => { + await waitFor(() => { + expect(document.querySelector('[role="dialog"]')).toBeTruthy() + }) + + await userEvent.click( + document.querySelector('[data-testid="cancel-button"]') as HTMLElement, + ) + + await waitFor(() => { + expect(document.querySelector('[role="dialog"]')).toBeNull() + }) + await waitFor(() => { + expect(document.activeElement).toHaveAttribute( + 'data-repo-card-id', + 'card-test', + ) + }) + }, +} + +/** + * A click on the overlay closes the note and focuses the card. + */ +export const OverlayReturnsFocusToTheCard: Story = { + args: { + isOpen: true, + onClose: fn(), + onSave: fn(), + cardId: toRepoCardId('card-test'), + initialNote: 'Test note', + initialLinks: [], + cardTitle: 'test/repo', + }, + render: (args) => , + play: async () => { + await waitFor(() => { + expect(document.querySelector('[role="dialog"]')).toBeTruthy() + }) + + const overlay = document.querySelector( + '[data-state="open"].fixed.inset-0', + ) as HTMLElement + await userEvent.click(overlay) + + await waitFor(() => { + expect(document.querySelector('[role="dialog"]')).toBeNull() + }) + await waitFor(() => { + expect(document.activeElement).toHaveAttribute( + 'data-repo-card-id', + 'card-test', + ) + }) + }, +} + +/** + * Save closes the note and focuses the card. + */ +export const SaveReturnsFocusToTheCard: Story = { + args: { + isOpen: true, + onClose: fn(), + onSave: fn().mockResolvedValue(undefined), + cardId: toRepoCardId('card-test'), + initialNote: 'Test note', + initialLinks: [], + cardTitle: 'test/repo', + }, + render: (args) => , + play: async () => { + await waitFor(() => { + expect(document.querySelector('[role="dialog"]')).toBeTruthy() + }) + + await userEvent.click( + document.querySelector('[data-testid="save-button"]') as HTMLElement, + ) + + await waitFor(() => { + expect(document.querySelector('[role="dialog"]')).toBeNull() + }) + await waitFor(() => { + expect(document.activeElement).toHaveAttribute( + 'data-repo-card-id', + 'card-test', + ) + }) + }, +} diff --git a/src/components/Modals/NoteModal.tsx b/src/components/Modals/NoteModal.tsx index df052c53..1105564d 100644 --- a/src/components/Modals/NoteModal.tsx +++ b/src/components/Modals/NoteModal.tsx @@ -15,9 +15,11 @@ import { DialogTitle, } from '@/components/ui/dialog' import { type ProjectLink } from '@/components/ui/editable-url-item' +import { useCardDialogFocusReturn } from '@/hooks/board/useCardDialogFocusReturn' import { useNoteModalDraft } from '@/hooks/board/useNoteModalDraft' import type { RepoCardId } from '@/lib/types/brands' import type { RepoIdentifier } from '@/lib/types/domain-primitives' +import { pendingButtonLabel } from '@/lib/utils/pending-button-label' interface NoteModalProps { /** Whether the modal is open */ @@ -104,12 +106,14 @@ export const NoteModal = memo(function NoteModal({ onClose, cardTitle, }) + const { handleCloseAutoFocus } = useCardDialogFocusReturn(cardId) return ( @@ -155,7 +159,7 @@ export const NoteModal = memo(function NoteModal({ disabled={isSaving || !isFormValid} data-testid="save-button" > - {isSaving ? 'Saving…' : 'Save'} + {pendingButtonLabel(isSaving, 'Save', 'Saving…')}
diff --git a/src/components/Modals/StatusListDialog.tsx b/src/components/Modals/StatusListDialog.tsx index 9878e5bf..601ffdd8 100644 --- a/src/components/Modals/StatusListDialog.tsx +++ b/src/components/Modals/StatusListDialog.tsx @@ -11,6 +11,7 @@ import { DialogFooter, DialogDescription, } from '@/components/ui/dialog' +import { FormError } from '@/components/ui/form-error' import { Input } from '@/components/ui/input' import { Label } from '@/components/ui/label' import type { StatusListDomain } from '@/lib/models/domain' @@ -136,7 +137,7 @@ const StatusListForm = memo(function StatusListForm({ type="button" onClick={() => setColor(preset.value)} data-color={preset.value} - className={`h-8 w-8 rounded-full border-2 transition-all ${color === preset.value ? 'border-foreground scale-110' : 'border-transparent'} `} + className={`h-8 w-8 rounded-full border-2 transition-[border-color,transform] ${color === preset.value ? 'border-foreground scale-110' : 'border-transparent'} `} style={{ backgroundColor: preset.value }} title={preset.name} aria-label={preset.name} @@ -164,7 +165,7 @@ const StatusListForm = memo(function StatusListForm({
{/* Error */} - {error &&

{error}

} +
)} - {error && ( - - )} +
) }) diff --git a/src/components/ui/font-color-toolbar-button.tsx b/src/components/ui/font-color-toolbar-button.tsx index e67e6faf..3108ec04 100644 --- a/src/components/ui/font-color-toolbar-button.tsx +++ b/src/components/ui/font-color-toolbar-button.tsx @@ -41,7 +41,7 @@ function ColorDropdownMenuItem({ size: 'icon', variant: 'outline', }), - 'border-muted my-1 flex size-6 items-center justify-center rounded-full border border-solid p-0 transition-all hover:scale-125', + 'border-muted my-1 flex size-6 items-center justify-center rounded-full border border-solid p-0 transition-transform hover:scale-125', !isBrightColor && 'border-transparent', isSelected && 'border-primary border-2', className, diff --git a/src/components/ui/form-error.tsx b/src/components/ui/form-error.tsx new file mode 100644 index 00000000..d542dba7 --- /dev/null +++ b/src/components/ui/form-error.tsx @@ -0,0 +1,47 @@ +'use client' + +import { memo } from 'react' + +/** + * Destructive inline error under a form field. + * + * One class and `role="alert"` for every refused write. Unconfirmed / + * unknown-outcome copy stays `text-warning` and must not use this. + * + * Called by rename, create-link, board settings, column edit, URL items, + * and Move to Another Board. + * + * @param message - The sentence to show. Empty or missing renders nothing. + * @param id - Optional id for `aria-describedby` on the field that failed. + * @param testId - Optional `data-testid`. + * + * @example + * ```ts + * + * ``` + */ +export const FormError = memo(function FormError({ + message, + id, + testId, +}: { + message?: string | null + /** For `aria-describedby` on the field that failed. */ + id?: string + testId?: string +}) { + if (!message) { + return null + } + + return ( + + ) +}) diff --git a/src/hooks/board/useCardDialogFocusReturn.ts b/src/hooks/board/useCardDialogFocusReturn.ts index 78c60e7d..0451ef59 100644 --- a/src/hooks/board/useCardDialogFocusReturn.ts +++ b/src/hooks/board/useCardDialogFocusReturn.ts @@ -18,8 +18,8 @@ import { /** * Keeps a keyboard user's place on the board when a card dialog closes: on the * card when it stays, on its neighbor when it was moved away or removed. - * Called by {@link MoveToAnotherBoardDialog} and by {@link OverflowMenu} for - * the Remove from Board confirmation. + * Called by {@link MoveToAnotherBoardDialog}, {@link NoteModal}, and by + * {@link OverflowMenu} for the Remove from Board confirmation. * * @param cardId - Card the dialog was opened for. * @returns diff --git a/src/lib/utils/pending-button-label.ts b/src/lib/utils/pending-button-label.ts new file mode 100644 index 00000000..4600dbed --- /dev/null +++ b/src/lib/utils/pending-button-label.ts @@ -0,0 +1,23 @@ +/** + * Idle or pending label for a submit control. + * + * Forms used to write `{isPending ? 'Creating…' : 'Create Board'}` by hand. + * {@link pendingButtonLabel} keeps the Title Case idle command and the + * ellipsis pending form in one place. Callers that show a spinner still + * render that icon themselves. + * + * @param isPending - Whether the request is in flight. + * @param idle - Title Case command, e.g. `Create Board`. + * @param pending - Ellipsis form, e.g. `Creating…`. + * @returns {@link pending} when in flight, otherwise {@link idle}. + * + * @example + * pendingButtonLabel(true, 'Create Board', 'Creating…') + */ +export function pendingButtonLabel( + isPending: boolean, + idle: string, + pending: string, +): string { + return isPending ? pending : idle +} diff --git a/src/tests/unit/components/Board/AddRepositoryCombobox.placements.test.tsx b/src/tests/unit/components/Board/AddRepositoryCombobox.placements.test.tsx index 91b42183..6f3c6288 100644 --- a/src/tests/unit/components/Board/AddRepositoryCombobox.placements.test.tsx +++ b/src/tests/unit/components/Board/AddRepositoryCombobox.placements.test.tsx @@ -1319,10 +1319,10 @@ describe('AddRepositoryCombobox with repositories placed elsewhere', () => { await findRepositoryOptions() // Act - await user.selectOptions( + await user.click( screen.getByRole('combobox', { name: 'Visibility filter' }), - 'Private', ) + await user.click(screen.getByRole('option', { name: 'Private' })) // Assert expect( @@ -1334,6 +1334,35 @@ describe('AddRepositoryCombobox with repositories placed elsewhere', () => { ).toBeNull() }) + // Value: protects=star counts use a lucide icon and Visibility is a Radix + // select; fails_when=the row paints ⭐ or the filter is a native ` next to a Radix select. (7) Star counts use an emoji where the rest of the app uses icons. - -**Why:** Polish (FINDING-013, FINDING-023 and the consistency review). None of them stops anyone; together they are why the app feels slightly different from page to page. - -**Context:** `src/components/Sidebar/Sidebar.tsx`, `src/hooks/board/useCardDialogFocusReturn.ts`, `src/app/settings/`, `src/components/Board/OverflowMenu.tsx`, `src/components/Board/StatusColumn.tsx`, `src/components/Board/AddRepositoryCombobox.tsx`. - -**Effort:** M -**Priority:** P4 -**Depends on:** None - ## Completed +- [x] **Smaller design gaps left from the 2026-10-12 review** + **Completed:** v0.7.21.0 (2026-10-18). Settings has the sidebar, a closed note returns focus to the card, forms share one error and pending label, transitions name the properties that change, the column menu has a separator, and the picker uses a Radix visibility list and a lucide star. - [x] **Preserve `?query` and `#hash` on silent refresh redirect** — [#177](https://github.com/laststance/gitbox/issues/177) **Completed:** v0.3.3.0 (2026-10-07). The repository catalog caller now forwards the full board destination, with unit coverage and a real-account OAuth round trip confirming both values survive. - [x] **Fix the hydration mismatch on the board page for returning users** diff --git a/VERSION b/VERSION index 39cb7e1e..46f2638f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.7.20.0 +0.7.21.0 diff --git a/package.json b/package.json index 21f0e0f1..6c15f60e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gitbox", - "version": "0.7.20", + "version": "0.7.21", "private": true, "type": "module", "description": "A web application for managing GitHub repositories in Kanban board format", From ecf46b66e08d89e2252d6460b493c6aecdadabbc Mon Sep 17 00:00:00 2001 From: Ryota Murakami Date: Sun, 18 Oct 2026 23:05:12 +0900 Subject: [PATCH 3/3] test: free laststance/nsx before Visibility picker assertions Seed card 4 already sits on the Test Board, so the picker had no public row and the new Visibility E2E timed out looking for nsx. --- e2e/logged-in/add-repository-combobox.spec.ts | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/e2e/logged-in/add-repository-combobox.spec.ts b/e2e/logged-in/add-repository-combobox.spec.ts index d01eb7c0..3d292f99 100644 --- a/e2e/logged-in/add-repository-combobox.spec.ts +++ b/e2e/logged-in/add-repository-combobox.spec.ts @@ -16,6 +16,7 @@ import type { Locator } from '@playwright/test' import { test, expect } from '../fixtures/coverage' import { BOARD_IDS, + CARD_IDS, TEST_USER_ID, WORK_PROJECTS_STATUS_IDS, createServiceRoleSupabaseClient, @@ -1438,6 +1439,17 @@ test.describe('AddRepositoryCombobox - Visibility and stars', () => { const BOARD_URL = `/board/${BOARD_IDS.testBoard}` + test.beforeEach(async () => { + // Seed card 4 is laststance/nsx. Every other public catalog fixture is + // already on this board, so the picker has no public row until this card + // is gone. + await freeNsxOnTestBoard() + }) + + test.afterEach(async () => { + await resetRepoCards() + }) + // Value: protects=Public hides the private fixture repo and Private shows it; // fails_when=the filter is ignored or a native select cannot be driven; // why_new=Visibility is now a Radix select; seam=none @@ -1480,7 +1492,7 @@ test.describe('AddRepositoryCombobox - Visibility and stars', () => { test('shows a lucide star next to the fixture star count', async ({ page, }) => { - // Arrange: laststance/nsx is not on the board and has 85 stars + // Arrange: laststance/nsx is free to add and has 85 stars await page.goto(BOARD_URL) await expect(page.getByText('Pending')).toBeVisible({ timeout: 15000 }) await page.getByRole('button', { name: /add repositories/i }).click() @@ -1492,12 +1504,31 @@ test.describe('AddRepositoryCombobox - Visibility and stars', () => { // Assert await expect(starCount).toBeVisible() - await expect(starCount).toHaveTextContent('85') + await expect(starCount).toHaveText('85') await expect(starCount.locator('svg')).toBeVisible() - await expect(starCount).not.toHaveTextContent('⭐') + await expect(starCount).not.toContainText('⭐') }) }) +/** + * Takes laststance/nsx off the Test Board so the picker can offer it. + * + * Seed card 4 is that repository. The Visibility and stars tests need a + * public catalog row that is not already placed; every other public fixture + * already sits on the board. + * + * @throws When the service-role delete fails. + */ +async function freeNsxOnTestBoard(): Promise { + const { error } = await createServiceRoleSupabaseClient() + .from('repocard') + .delete() + .eq('id', CARD_IDS.card4) + if (error) { + throw new Error(`freeNsxOnTestBoard failed: ${error.message}`) + } +} + /** * Places repositories on the Work Projects board, bypassing the app, so the * picker of another board lists them under "Already placed elsewhere".