diff --git a/.changeset/list-link-column-real-anchor.md b/.changeset/list-link-column-real-anchor.md new file mode 100644 index 000000000..222477d71 --- /dev/null +++ b/.changeset/list-link-column-real-anchor.md @@ -0,0 +1,32 @@ +--- +'@object-ui/plugin-grid': patch +'@object-ui/app-shell': patch +--- + +fix(plugin-grid): the list link column renders a real anchor when the host publishes record URLs + +The list's `link: true` column (and the auto-linked primary field) rendered as +a `span role="link"` with no `href`, navigating only through a click handler. +So the surface users actually open records from had none of a link's native +affordances — no middle-click / ⌘-click open-in-new-tab, no "copy link +address", no hover status-bar URL — and `role="link"` without an href is a +weaker contract for assistive tech than a real anchor. It was also the odd one +out: the previous release gave record-detail and related-list lookup VALUES +real anchors, leaving the list column as the weakest of the three surfaces. + +`LinkCell` now renders a real `` with the same click split: a plain +left click is prevented and handed to the existing in-app navigation, so drawer +/ modal / page behavior is completely unchanged, while modifier and +middle-clicks are left to the browser. + +The URL is not assembled in the grid. The object list page publishes its own +record-URL builder through `RelatedRecordActionsContext.recordHref` — the same +seam the lookup links use, and the same expression its "open in new window" +action already navigated with, so the anchor and that action cannot address +different records. A host that publishes no URL renders exactly what it +rendered before: the Studio designer, embedded renderers and standalone grids +are untouched. + +Neither package's published `dist/index.d.ts` changes (measured both ways — +byte-identical), so this is a patch on both: the list host's new helpers are +module-level exports behind a barrel that re-exports only `ObjectView`. diff --git a/packages/app-shell/src/views/ObjectView.listRecordHref.test.ts b/packages/app-shell/src/views/ObjectView.listRecordHref.test.ts new file mode 100644 index 000000000..63db8415e --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.listRecordHref.test.ts @@ -0,0 +1,124 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#4490, the HOST half — the object list page publishes its record-URL + * builder so the grid's link column can render a real anchor. + * + * The seam was the measured question. `RelatedRecordActionsProvider` mounted + * only on the record DETAIL body (`RelatedRecordActionsBridge`), which is why + * PR #4489's anchors reached lookup values there and never the list page. Of + * the two candidate seams — mount the existing provider on the list host, or + * thread an href callback through the grid's navigation context — the first + * needs no new package dependency and no new schema key: app-shell and + * plugin-grid both already depend on `@object-ui/react`, where the context and + * its `recordHref` (added by #4489) live. + * + * What is pinned here is the part that is this file's own: the URL is built + * ONCE, by the host, from the same expression its "open in new window" action + * has always used — so the anchor a user middle-clicks and the tab the row's + * ⌘-click opens address the same record by construction, not by coincidence. + * + * REVERSE VERIFICATION — direction predicted, then measured. These are pure + * functions that do not exist on `origin/main`, so reverting the source does + * not produce a value mismatch: the import fails and the whole file dies before + * an assertion runs. Stated plainly because it bounds what this file proves — + * it pins the builder's CONTRACT going forward (scoping, encoding, the `/view/` + * strip); the case that discriminates old behavior from new is the anchor + * itself, in `plugin-grid/src/__tests__/ObjectGrid.linkCellAnchor.test.tsx`. + */ +import { describe, it, expect, vi } from 'vitest'; + +import { listRecordDetailUrl, listRecordActionsValue } from './ObjectView'; + +describe('listRecordDetailUrl — one record-route shape for the list surface (#4490)', () => { + it('strips the trailing view segment and appends the record route', () => { + expect(listRecordDetailUrl('/apps/demo/crm_lead/view/all', 'r1')).toBe( + '/apps/demo/crm_lead/record/r1', + ); + }); + + it('works on the object page with no view segment', () => { + expect(listRecordDetailUrl('/apps/demo/crm_lead', 'r1')).toBe( + '/apps/demo/crm_lead/record/r1', + ); + }); + + it('encodes the record id', () => { + expect(listRecordDetailUrl('/apps/demo/crm_lead', 'a/b c')).toBe( + '/apps/demo/crm_lead/record/a%2Fb%20c', + ); + }); + + it('numeric ids survive as ids, not as coincidental path segments', () => { + expect(listRecordDetailUrl('/apps/demo/crm_lead/view/mine', 42)).toBe( + '/apps/demo/crm_lead/record/42', + ); + }); +}); + +describe('listRecordActionsValue — what the list page publishes (#4490)', () => { + const pathname = () => '/apps/demo/crm_lead/view/all'; + + it('publishes an href for the object this view lists', () => { + const value = listRecordActionsValue('crm_lead', vi.fn(), pathname); + expect(value.recordHref!('crm_lead', 'r1')).toBe('/apps/demo/crm_lead/record/r1'); + }); + + it('publishes NO href for any other object — this builder cannot name one', () => { + const value = listRecordActionsValue('crm_lead', vi.fn(), pathname); + // A lookup cell pointing at a third object must render as the plain value, + // which is what `null` means to the consumer. The console-wide builder is + // the detail page's bridge, which has the routable-object set this page + // does not — inventing a URL here is exactly the #4472 mistake. + expect(value.recordHref!('crm_account', 'acc-1')).toBeNull(); + }); + + it('publishes NO href for a record with no id', () => { + const value = listRecordActionsValue('crm_lead', vi.fn(), pathname); + expect(value.recordHref!('crm_lead', '')).toBeNull(); + }); + + it('openRecord routes through the host own record navigation', () => { + const openPage = vi.fn(); + const value = listRecordActionsValue('crm_lead', openPage, pathname); + + value.openRecord!('crm_lead', 'r1'); + expect(openPage).toHaveBeenCalledTimes(1); + expect(openPage).toHaveBeenCalledWith('r1'); + }); + + it('openRecord is a no-op for an object this page cannot address', () => { + const openPage = vi.fn(); + const value = listRecordActionsValue('crm_lead', openPage, pathname); + + value.openRecord!('crm_account', 'acc-1'); + expect(openPage).not.toHaveBeenCalled(); + }); + + it('reads the pathname at CALL time, so a view switch cannot leave a stale base', () => { + let current = '/apps/demo/crm_lead/view/all'; + const value = listRecordActionsValue('crm_lead', vi.fn(), () => current); + expect(value.recordHref!('crm_lead', 'r1')).toBe('/apps/demo/crm_lead/record/r1'); + + current = '/apps/other/crm_lead/view/mine'; + expect(value.recordHref!('crm_lead', 'r1')).toBe('/apps/other/crm_lead/record/r1'); + }); + + it('resolves NO related-list handlers — a list page has no child collection', () => { + const value = listRecordActionsValue('crm_lead', vi.fn(), pathname); + // The context reads an omitted handler as "capability unavailable", i.e. + // the same read-only outcome a consumer gets with no provider at all. So + // mounting this provider grants no affordance that was not there before. + expect(value.resolve({ objectName: 'crm_lead' })).toEqual({}); + // Stable identity, so a consumer memoizing on it does not churn. + expect(value.resolve({ objectName: 'crm_lead' })).toBe( + value.resolve({ objectName: 'anything' }), + ); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 28f3f555f..b1832cfd6 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -70,7 +70,8 @@ import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n'; import { usePermissions } from '@object-ui/permissions'; import { useAuth, useIsWorkspaceAdmin } from '@object-ui/auth'; import { useRealtimeSubscription, useConflictResolution } from '@object-ui/collaboration'; -import { ActionProvider, useNavigationOverlay, SchemaRenderer, useActionTextLocalizer } from '@object-ui/react'; +import { ActionProvider, useNavigationOverlay, SchemaRenderer, useActionTextLocalizer, RelatedRecordActionsProvider } from '@object-ui/react'; +import type { RelatedRecordActionsValue, RelatedRecordHandlers } from '@object-ui/react'; import { toast } from 'sonner'; import { useConsoleActionRuntime } from '../hooks/useConsoleActionRuntime'; import { useEnvironmentEntitlements } from '../environment/useEnvironmentEntitlements'; @@ -161,6 +162,83 @@ export function timelineViewOptions(viewDef: any, objectDef: any): Record void, + getPathname: () => string = () => window.location.pathname, +): RelatedRecordActionsValue { + const addressable = (targetObjectName: string, recordId: string | number) => + targetObjectName === objectName && recordId != null && recordId !== ''; + return { + resolve: () => NO_RELATED_HANDLERS, + recordHref: (targetObjectName, recordId) => + addressable(targetObjectName, recordId) + ? listRecordDetailUrl(getPathname(), recordId) + : null, + openRecord: (targetObjectName, recordId) => { + if (!addressable(targetObjectName, recordId)) return; + openRecordPage(recordId); + }, + }; +} + export function defaultListColumnsFromObject( objectDef: any, limit = 5, @@ -1404,9 +1482,10 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an const handleNavOverlayNavigate = useCallback( (recordId: string | number, action?: string) => { if (action === 'new_window') { - // Open record detail in a new browser tab with Console-correct URL - const basePath = window.location.pathname.replace(/\/view\/.*$/, ''); - window.open(`${basePath}/record/${encodeURIComponent(String(recordId))}`, '_blank'); + // Open record detail in a new browser tab with Console-correct URL. + // Same builder the list's link column publishes as its href + // (objectui#4490) — one record-route shape for this surface. + window.open(listRecordDetailUrl(window.location.pathname, recordId), '_blank'); return; } // Default: navigate to record detail page. @@ -1428,6 +1507,20 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an }, [navigate, viewId, location.pathname, location.search, objectDef, activeView?.name, activeView?.label, viewLabel, objectLabel] ); + /** + * The list surface's half of the record-link mechanism (objectui#4490): + * publish this page's record-URL builder so the grid's `link: true` column + * can render a real anchor instead of a click-only `span role="link"`. + * See {@link listRecordActionsValue} for what this does and does NOT claim. + * + * The record drawer/overlay below mounts `RecordDetailView`, which brings + * its own `RelatedRecordActionsBridge` around the whole detail body — that + * nearer provider keeps owning the detail surface, exactly as before. + */ + const listRecordActions = useMemo( + () => listRecordActionsValue(objectDef.name, handleNavOverlayNavigate), + [objectDef.name, handleNavOverlayNavigate], + ); const navOverlay = useNavigationOverlay({ navigation: detailNavigation, objectName: objectDef.name, @@ -1954,6 +2047,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an return ( +
{/* 1. Header with breadcrumb + description. The managed-by badge sits inline with the title so the @@ -2344,6 +2438,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an )}
{actionRuntime.dialogs} + ); } \ No newline at end of file diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index cbc0c478f..3f7938018 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -25,7 +25,7 @@ import React, { useEffect, useState, useCallback, useMemo } from 'react'; import type { ObjectGridSchema, DataSource, ListColumn, ViewData, TableSortItem, DataTableSchema } from '@object-ui/types'; import { isSystemManagedField } from '@object-ui/types'; import type { I18nLabel } from '@objectstack/spec/ui'; -import { SchemaRenderer, useDataScope, useNavigationOverlay, useAction, useSafeFieldLabel, usePredicateScope } from '@object-ui/react'; +import { SchemaRenderer, useDataScope, useNavigationOverlay, useAction, useSafeFieldLabel, usePredicateScope, useRelatedRecordActions } from '@object-ui/react'; import { createSafeTranslation } from '@object-ui/i18n'; import { getCellRenderer, resolveCellRendererType, formatCurrency, formatCompactCurrency, formatDate, formatPercent, humanizeLabel, getBadgeColorClasses, FieldEditWidget, hasFieldEditWidget, DISCRETE_EDIT_TYPES, coerceToSafeValue } from '@object-ui/fields'; import { useLocalization, resolveFieldCurrency } from '@object-ui/i18n'; @@ -80,36 +80,126 @@ export function parseSchemaSort(sort: unknown): TableSortItem[] { return items; } +/** + * The class list both faces of {@link LinkCell} wear — the anchor is a visual + * no-op against the span it upgrades, so a list does not change appearance + * depending on whether its host publishes record URLs. + */ +const LINK_CELL_CLASS = + 'text-primary font-medium underline-offset-4 hover:underline cursor-pointer truncate block max-w-full'; + +/** + * A row's primary key — the value the host's record-URL builder addresses. + * Mirrors what `useNavigationOverlay.handleClick` reads off the same row, so + * the anchor's href and the click path can never point at different records. + */ +function rowRecordId(row: unknown): string | number | undefined { + const rec = row as Record | null | undefined; + const id = rec?.id ?? rec?._id; + return typeof id === 'string' || typeof id === 'number' ? id : undefined; +} + // Clickable text cell that can safely contain other interactive content // (e.g., EmailCellRenderer's copy button). Using