Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,16 @@ import {
interface SaveViewModalProps {
open: boolean
onOpenChange: (open: boolean) => void
/** Pre-filled when renaming an existing view; empty when saving a new one. */
/** Pre-filled when renaming an existing view; empty when creating a new one. */
initialName?: string
/** `new` starts blank and is configured after; `create` captures what is
* already applied; `rename` retitles an existing view. */
mode: 'new' | 'create' | 'rename'
/** `new` starts blank and is configured after; `rename` retitles an existing view. */
mode: 'new' | 'rename'
onSubmit: (name: string) => void
isSubmitting: boolean
}

/**
* Names a view — used both for "Save as view" and for renaming an existing one.
* Names a new view or renames an existing one.
* A view name is free-form (no identifier rules), so the only guard is emptiness.
*/
export function SaveViewModal({
Expand All @@ -43,7 +42,7 @@ export function SaveViewModal({
}

const trimmed = name.trim()
const title = mode === 'new' ? 'New view' : mode === 'create' ? 'Save as view' : 'Rename view'
const title = mode === 'new' ? 'New view' : 'Rename view'

const handleSubmit = () => {
if (!trimmed || isSubmitting) return
Expand All @@ -68,7 +67,14 @@ export function SaveViewModal({
onCancel={() => onOpenChange(false)}
cancelDisabled={isSubmitting}
primaryAction={{
label: isSubmitting ? 'Saving...' : 'Save',
label:
mode === 'new'
? isSubmitting
? 'Creating...'
: 'Create'
: isSubmitting
? 'Saving...'
: 'Save',
onClick: handleSubmit,
disabled: !trimmed || isSubmitting,
}}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* @vitest-environment jsdom
*/
Comment on lines +1 to +3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unnecessary jsdom test environment

This test only uses renderToStaticMarkup and string assertions, so selecting jsdom initializes an unnecessary browser-like environment and obscures the test's actual runtime requirements.

Suggested change
/**
* @vitest-environment jsdom
*/

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

import { renderToStaticMarkup } from 'react-dom/server'
import { describe, expect, it, vi } from 'vitest'
import type { TableViewWire } from '@/lib/api/contracts/tables'
import { ViewsMenu } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/views-menu/views-menu'

const DEFAULT_VIEW: TableViewWire = {
id: 'view-default',
tableId: 'table-1',
name: 'Default',
config: {},
isDefault: true,
createdBy: 'user-1',
createdAt: new Date('2026-08-15T01:00:00.000Z'),
updatedAt: new Date('2026-08-15T01:00:00.000Z'),
}

function renderMenu(views: TableViewWire[], activeViewId: string | null): string {
return renderToStaticMarkup(
<ViewsMenu
views={views}
activeViewId={activeViewId}
onSelect={vi.fn()}
onRename={vi.fn()}
onDelete={vi.fn()}
onNewView={vi.fn()}
canEdit
/>
)
}

describe('ViewsMenu', () => {
it('shows the persisted default while its URL selection is being adopted', () => {
const markup = renderMenu([DEFAULT_VIEW], null)

expect(markup).toContain('Default')
expect(markup).not.toContain('>View<')
})

it('shows All only for a legacy table without a persisted default', () => {
const markup = renderMenu([], null)

expect(markup).toContain('All')
expect(markup).not.toContain('>View<')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import {
} from '@sim/emcn'
import { Check, Pencil, Plus, Trash } from '@sim/emcn/icons'
import type { TableViewWire } from '@/lib/api/contracts/tables'
import { resolveTableViewSelection } from '@/app/workspace/[workspaceId]/tables/[tableId]/view-state'

/** Label for the built-in unfiltered state. Not a stored row — `null` view id. */
/** Legacy label for tables that do not yet have a persisted default view. */
export const ALL_ROWS_VIEW_LABEL = 'All'

/** Matches the breadcrumb location popover's hover-intent grace period. */
Expand All @@ -29,7 +30,7 @@ const VIEW_ACTION_SLOT_PX = 22

interface ViewsMenuProps {
views: TableViewWire[]
/** `null` selects the built-in "All" state. */
/** `null` selects the legacy "All" state while a table awaits backfill. */
activeViewId: string | null
onSelect: (viewId: string | null) => void
onRename: (viewId: string) => void
Expand All @@ -41,8 +42,8 @@ interface ViewsMenuProps {
}

/**
* View switcher for the table options bar. Reads "View" until one is selected,
* then carries the active view's name.
* View switcher for the table options bar. Carries the active view's name, or
* resolves an absent selection to the persisted default while the URL catches up.
*
* Opens on hover-intent like the header's breadcrumb location popover, so the
* list of views is discoverable without a click.
Expand All @@ -59,8 +60,9 @@ export const ViewsMenu = memo(function ViewsMenu({
const [open, setOpen] = useState(false)
const closeTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)

const activeView = activeViewId ? views.find((view) => view.id === activeViewId) : undefined
const label = activeView?.name ?? 'View'
const { activeView, defaultView } = resolveTableViewSelection(views, activeViewId)
const hasDefaultView = defaultView !== null
const label = activeView?.name ?? ALL_ROWS_VIEW_LABEL

const cancelScheduledClose = () => {
if (closeTimeoutRef.current) {
Expand Down Expand Up @@ -132,11 +134,13 @@ export const ViewsMenu = memo(function ViewsMenu({
Views
</PopoverSection>
<div className='flex flex-col gap-0.5'>
<ViewRow
label={ALL_ROWS_VIEW_LABEL}
isActive={activeViewId === null}
onSelect={() => runAndClose(() => onSelect(null))}
/>
{!hasDefaultView && (
<ViewRow
label={ALL_ROWS_VIEW_LABEL}
isActive={activeViewId === null}
onSelect={() => runAndClose(() => onSelect(null))}
/>
)}
{views.map((view) => (
<ViewRow
key={view.id}
Expand Down
Loading
Loading