From a64e78e68afb5430d7040dd57a2c06526dc1e8ce Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:38:13 +0000 Subject: [PATCH 1/2] fix(components,plugin-list): the Sort picker lists an in-use-only field as removable, never as a new choice (objectui#11943) SortBuilder's fields entries take an optional `disabled`: the entry is passed to SelectItem as an unavailable option, "Add sort" seeds the first entry that is not disabled and is disabled when none is. ListView's sortFields flags every entry it keeps only because the current sort names it: unreadable, platform-refused, or relational. Claude-Session: https://claude.ai/code/session_01DBZ9bntPZ7VKyQNtJeNsgw Co-authored-by: Claude --- ...sort-builder-disabled-entry-11943.test.tsx | 197 +++++++++++++++ .../components/src/custom/sort-builder.tsx | 20 +- packages/plugin-list/src/ListView.tsx | 30 ++- .../ListView.sortRemovableOnly-11943.test.tsx | 234 ++++++++++++++++++ 4 files changed, 467 insertions(+), 14 deletions(-) create mode 100644 packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx create mode 100644 packages/plugin-list/src/__tests__/ListView.sortRemovableOnly-11943.test.tsx diff --git a/packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx b/packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx new file mode 100644 index 0000000000..363d2ebbf4 --- /dev/null +++ b/packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx @@ -0,0 +1,197 @@ +/** + * 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#11943 — `SortBuilder` honours `disabled` on a `fields` entry. + * + * A host sometimes has to list a field it must not offer: the field a row + * already sorts by, kept so that row is not blank and can be removed, while no + * other row may pick it (the ListView Sort picker's in-use exception). Before + * this the entries were `{ value, label }` only, and one shared list fed every + * row's dropdown and "Add sort", so a kept field was choosable everywhere. + * + * Every case reads the real `SortBuilder` and its real Radix `Select`, and sits + * beside a control that runs the same interaction on the same entry WITHOUT the + * flag: a negative such as "onChange was not called" is otherwise satisfied by a + * dropdown that never opened or an option that was never there. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import React from 'react'; +import { cleanup, render, screen, fireEvent, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { SortBuilder, type SortBuilderProps, type SortItem } from '../custom/sort-builder'; + +type Fields = NonNullable; + +/** `secret` is the kept entry: flagged here, and listed FIRST so "Add sort" would seed it. */ +const FLAGGED: Fields = [ + { value: 'secret', label: 'Secret', disabled: true }, + { value: 'title', label: 'Title' }, + { value: 'status', label: 'Status' }, +]; + +/** The control: the same entries, in the same order, with no flag. */ +const PLAIN: Fields = FLAGGED.map(({ value, label }) => ({ value, label })); + +type Row = Pick; + +/** Holds the value the way a host does, so a change is rendered back. */ +function Harness({ fields, initial, onChange }: { fields: Fields; initial: Row[]; onChange: (rows: Row[]) => void }) { + const [value, setValue] = React.useState(() => + initial.map((row, i) => ({ id: `r${i}`, ...row })), + ); + return ( + { + onChange(next.map(({ field, order }) => ({ field, order }))); + setValue(next); + }} + /> + ); +} + +function mount(fields: Fields, initial: Row[] = []) { + const onChange = vi.fn(); + render(); + return onChange; +} + +/** Each row is `[field select, direction select]`; these are the field selects. */ +function fieldTriggers(): HTMLElement[] { + return screen.getAllByRole('combobox').filter((_, i) => i % 2 === 0); +} + +/** Open a row's field dropdown and return its option elements. */ +async function open(trigger: HTMLElement): Promise { + fireEvent.click(trigger); + const listbox = await screen.findByRole('listbox'); + return within(listbox).getAllByRole('option'); +} + +function option(options: HTMLElement[], label: string): HTMLElement { + const found = options.find((o) => o.textContent?.trim() === label); + if (!found) throw new Error(`no option "${label}"`); + return found; +} + +const asc = (field: string): Row => ({ field, order: 'asc' }); + +afterEach(() => { + cleanup(); +}); + +describe('SortBuilder honours `disabled` on a fields entry (objectui#11943)', () => { + it('renders a flagged entry as an unavailable option, and the same entry unflagged as an ordinary one', async () => { + mount(FLAGGED, [asc('title')]); + let options = await open(fieldTriggers()[0]); + expect(options.map((o) => o.textContent?.trim())).toEqual(['Secret', 'Title', 'Status']); + expect(option(options, 'Secret')).toHaveAttribute('aria-disabled', 'true'); + expect(option(options, 'Secret')).toHaveAttribute('data-disabled'); + expect(option(options, 'Title')).not.toHaveAttribute('aria-disabled'); + expect(option(options, 'Title')).not.toHaveAttribute('data-disabled'); + + cleanup(); + mount(PLAIN, [asc('title')]); + options = await open(fieldTriggers()[0]); + expect(option(options, 'Secret')).not.toHaveAttribute('aria-disabled'); + expect(option(options, 'Secret')).not.toHaveAttribute('data-disabled'); + }); + + it('a flagged entry cannot be chosen by click; unflagged, the same click chooses it', async () => { + let onChange = mount(FLAGGED, [asc('title')]); + let secret = option(await open(fieldTriggers()[0]), 'Secret'); + fireEvent.click(secret); + fireEvent.pointerDown(secret, { pointerType: 'mouse' }); + fireEvent.pointerUp(secret, { pointerType: 'mouse' }); + expect(onChange).not.toHaveBeenCalled(); + expect(fieldTriggers()[0]).toHaveTextContent('Title'); + + cleanup(); + onChange = mount(PLAIN, [asc('title')]); + secret = option(await open(fieldTriggers()[0]), 'Secret'); + fireEvent.click(secret); + expect(onChange).toHaveBeenLastCalledWith([asc('secret')]); + expect(fieldTriggers()[0]).toHaveTextContent('Secret'); + }); + + it('a flagged entry cannot be chosen from the keyboard; unflagged, the same keys choose it', async () => { + // Enter on the option itself. + let onChange = mount(FLAGGED, [asc('title')]); + fireEvent.keyDown(option(await open(fieldTriggers()[0]), 'Secret'), { key: 'Enter' }); + expect(onChange).not.toHaveBeenCalled(); + + cleanup(); + onChange = mount(PLAIN, [asc('title')]); + fireEvent.keyDown(option(await open(fieldTriggers()[0]), 'Secret'), { key: 'Enter' }); + expect(onChange).toHaveBeenLastCalledWith([asc('secret')]); + + // Typeahead on the closed trigger, which picks the next entry after the + // current one whose label starts with the typed key. From `Status` the + // only other "s" entry is `Secret`. + cleanup(); + onChange = mount(FLAGGED, [asc('status')]); + fireEvent.keyDown(fieldTriggers()[0], { key: 's' }); + expect(onChange).not.toHaveBeenCalled(); + expect(fieldTriggers()[0]).toHaveTextContent('Status'); + + cleanup(); + onChange = mount(PLAIN, [asc('status')]); + fireEvent.keyDown(fieldTriggers()[0], { key: 's' }); + expect(onChange).toHaveBeenLastCalledWith([asc('secret')]); + }); + + it('a row whose field is a flagged entry shows its label, and can be changed and removed', async () => { + let onChange = mount(FLAGGED, [asc('secret'), asc('status')]); + // Not blank: the label of a value with no entry at all renders nothing, + // which is what this assertion would read if the flag dropped the entry. + expect(fieldTriggers()[0]).toHaveTextContent('Secret'); + + // Changed to another field. + fireEvent.click(option(await open(fieldTriggers()[0]), 'Title')); + expect(onChange).toHaveBeenLastCalledWith([asc('title'), asc('status')]); + expect(fieldTriggers()[0]).toHaveTextContent('Title'); + + // Removed: the row's only button is its remove control. + cleanup(); + onChange = mount(FLAGGED, [asc('secret'), asc('status')]); + const row = screen.getByText('Sort by').parentElement as HTMLElement; + fireEvent.click(within(row).getByRole('button')); + expect(onChange).toHaveBeenLastCalledWith([asc('status')]); + }); + + it('control: a value with no entry renders a blank row, which the flagged row above is not', () => { + mount(FLAGGED, [asc('nowhere')]); + expect(fieldTriggers()[0].textContent?.trim()).toBe(''); + }); + + it('"Add sort" seeds the first entry that is not flagged; unflagged, it seeds the first entry', () => { + let onChange = mount(FLAGGED); + fireEvent.click(screen.getByRole('button', { name: /add sort/i })); + expect(onChange).toHaveBeenLastCalledWith([asc('title')]); + + cleanup(); + onChange = mount(PLAIN); + fireEvent.click(screen.getByRole('button', { name: /add sort/i })); + expect(onChange).toHaveBeenLastCalledWith([asc('secret')]); + }); + + it('"Add sort" is disabled when no entry can be chosen, and enabled when one can', () => { + mount(FLAGGED.map((f) => ({ ...f, disabled: true })), [asc('secret')]); + expect(screen.getByRole('button', { name: /add sort/i })).toBeDisabled(); + + cleanup(); + mount(FLAGGED, [asc('secret')]); + expect(screen.getByRole('button', { name: /add sort/i })).toBeEnabled(); + + cleanup(); + mount([]); + expect(screen.getByRole('button', { name: /add sort/i })).toBeDisabled(); + }); +}); diff --git a/packages/components/src/custom/sort-builder.tsx b/packages/components/src/custom/sort-builder.tsx index 8855a2cca7..ec3a3c7bf4 100644 --- a/packages/components/src/custom/sort-builder.tsx +++ b/packages/components/src/custom/sort-builder.tsx @@ -45,9 +45,17 @@ export interface SortItem extends SpecSortItem { } export interface SortBuilderProps { - fields?: Array<{ + fields?: Array<{ value: string label: string + /** + * Listed but not choosable (objectui#11943). A disabled entry still names + * a row whose current field it is, so that row shows its label and can be + * changed or removed, but no row's dropdown lets the user pick it and + * "Add sort" never seeds it. "Add sort" seeds the first entry that is not + * disabled, and is itself disabled when every entry is. + */ + disabled?: boolean }>; value?: SortItem[]; onChange?: (value: SortItem[]) => void; @@ -89,10 +97,14 @@ export function SortBuilder({ onChange?.(newItems); }; + // A new row starts on the first field the user may choose: a disabled + // entry is only there for the row that already names it. + const firstChoosable = fields.find((f) => !f.disabled); + const addItem = () => { const newItem: SortItem = { id: crypto.randomUUID(), - field: fields[0]?.value || "", + field: firstChoosable?.value || "", order: 'asc', }; handleChange([...items, newItem]); @@ -124,7 +136,7 @@ export function SortBuilder({ {fields.map(f => ( - {f.label} + {f.label} ))} @@ -159,7 +171,7 @@ export function SortBuilder({ size="sm" onClick={addItem} className="h-8" - disabled={fields.length === 0} + disabled={!firstChoosable} > {t('sortBuilder.addSort')} diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 0edf2348fd..dcd8203540 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -3188,8 +3188,8 @@ export const ListView = React.forwardRef(({ // it can. The Filter panel's list and the Sort picker's list ask through // `canReadField` (objectui#11925, objectui#11943). One exception: the Sort // picker keeps a field the current sort already names, so its row can be - // removed. Until objectui#11943's follow-up that field is also still - // choosable in the picker's other rows. + // removed. It lists that field disabled, so no other row and no "Add sort" + // can choose it. const effectiveFields = React.useMemo(() => { // Defensive: `columns` is `string[] | ListColumn[]`, but metadata is // user-authored — anything non-array degrades to "no declared columns". @@ -4123,12 +4123,18 @@ export const ListView = React.forwardRef(({ // user may not read is not offered: the server answers a sort on it with a // 403, and the list blanks to its no-access state. A dropped field does not // raise the relational hint either; the hint explains a missing relation the - // user could otherwise read. The in-use exception covers this rule too. A + // user could otherwise read. The in-use exception covers this rule too: a // field the current sort already names (a stored or URL sort) stays listed, - // exactly as the two rules above would list it and with no mark, so its row - // is not blank and can be removed. Until objectui#11943's follow-up it is - // also still choosable in every other row, and by "Add sort" when it comes - // first: listing it as removable only needs a `SortBuilder` change. + // so its row is not blank and can be removed. + // + // An entry the exception alone keeps is listed REMOVABLE ONLY + // (objectui#11943): it carries `disabled`, which `SortBuilder` renders as an + // unavailable option. Its own row still shows its label and can be changed + // to another field or removed, but no row's dropdown offers it as a choice + // and "Add sort" never seeds it. That holds for each reason the exception + // keeps a field: unreadable, relational, or refused by the platform (or by + // the type read when no projection is served). A field the two rules list + // anyway carries no flag, whether or not the sort names it. // // ONE read of the served projection, for BOTH legs below — the list this // picker renders, and the sort it emits for a host to persist. Read twice, @@ -4143,21 +4149,25 @@ export const ListView = React.forwardRef(({ const { sortFields, sortHasRelationalField } = React.useMemo(() => { const inUse = new Set(currentSort.map((item) => item.field).filter(Boolean)); let excluded = false; - const fields: Array<{ value: string; label: string }> = []; + const fields: Array<{ value: string; label: string; disabled?: boolean }> = []; for (const field of candidateFields) { - if (!inUse.has(field.value) && !canReadField(perms, schema.objectName, field.value)) continue; + const readable = canReadField(perms, schema.objectName, field.value); + if (!readable && !inUse.has(field.value)) continue; const relational = EXPANDABLE_FIELD_TYPES.has(field.type); const platformSortable = platformSortability ? isPlatformSortableField(platformSortability, field.value) : !UNMATERIALIZED_FIELD_TYPES.has(field.type); - if (!relational && platformSortable) { + if (readable && !relational && platformSortable) { fields.push({ value: field.value, label: field.label }); continue; } if (inUse.has(field.value)) { + // Listed only because the current sort names it: `disabled`, so its + // own row shows it and can drop it, and nothing can choose it anew. fields.push({ value: field.value, label: relational ? `${field.label} ${t('list.sortByIdSuffix')}` : field.label, + disabled: true, }); continue; } diff --git a/packages/plugin-list/src/__tests__/ListView.sortRemovableOnly-11943.test.tsx b/packages/plugin-list/src/__tests__/ListView.sortRemovableOnly-11943.test.tsx new file mode 100644 index 0000000000..67d4927c29 --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.sortRemovableOnly-11943.test.tsx @@ -0,0 +1,234 @@ +/** + * 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#11943, the removable-only half: a field the Sort picker keeps ONLY + * because the current sort names it is listed disabled. + * + * The triage ruling: such a field "stays visible only as removable, marked + * unavailable and never offered as a new choice". The picker keeps a field for + * that reason in three cases, and each is measured here on the real + * `SortBuilder`: + * + * - UNREADABLE: field-level read denies it (`canReadField`, objectui#11943). + * - PLATFORM-REFUSED: the served projection refuses to order by it (#6455). + * - RELATIONAL: a lookup, listed as ordering by ID (objectui#4243). + * + * In each case the field is declared FIRST on the object, so an "Add sort" that + * still seeded the first entry would seed it. The controls: a field the picker + * lists anyway carries no flag, in use or not, and with full read and no + * served projection the same unreadable and refused fields are ordinary + * options again. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { cleanup, render, screen, fireEvent, waitFor, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { resolveObjectSortability } from '@objectstack/spec/api'; +import { attachObjectSortability } from '@object-ui/core'; +import type { DataSource, ListViewSchema } from '@object-ui/types'; +import { SchemaRendererProvider } from '@object-ui/react'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; +import { ListView } from '../ListView'; + +const OBJECT = 'fls_ticket'; + +const FIELD_DEFS: Record> = { + secret_note: { type: 'text', label: 'Secret Note' }, + remote_status: { type: 'text', label: 'Remote Status' }, + owner: { type: 'lookup', label: 'Owner', reference: 'sys_user' }, + title: { type: 'text', label: 'Title' }, + priority: { type: 'number', label: 'Priority' }, +}; + +/** The object definition with `first` declared first, the rest in `FIELD_DEFS` order. */ +function definition(first: string) { + const fields: Record> = { [first]: FIELD_DEFS[first] }; + for (const [name, def] of Object.entries(FIELD_DEFS)) if (name !== first) fields[name] = def; + return { name: OBJECT, label: 'Ticket', fields }; +} + +function answer(fields: MePermissionsResponse['fields']): MePermissionsResponse { + return { + authenticated: true, + userId: 'u1', + tenantId: null, + roles: [], + permissionSets: ['fls_member'], + objects: { [OBJECT]: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false } }, + fields, + }; +} + +const RESTRICTED = answer({ [`${OBJECT}.secret_note`]: { readable: false, editable: false } }); +const FULL_READ = answer({}); + +/** The platform's own resolver, with `remote_status` refused, as #6455's pins serve it. */ +function attachProjection(def: ReturnType) { + const resolved = resolveObjectSortability(def) as { fields: Record }; + const fields: Record = { ...resolved.fields }; + // The base the refusal is measured against: every other field is sortable. + for (const name of ['secret_note', 'owner', 'title', 'priority']) { + expect(fields[name]).toEqual({ sortable: true }); + } + fields.remote_status = { sortable: false }; + attachObjectSortability(def, { fields }); + return def; +} + +function mount( + { first, sort, perms = RESTRICTED, projection = true }: { + first: string; + sort: string[]; + perms?: MePermissionsResponse; + projection?: boolean; + }, +) { + const dataSource = { + find: vi.fn().mockResolvedValue({ data: [], total: 0 }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => { + const def = definition(first); + return projection ? attachProjection(def) : def; + }), + }; + const schema = { + type: 'list-view', + objectName: OBJECT, + viewType: 'grid', + columns: Object.keys(FIELD_DEFS).map((field) => ({ field })), + sort: sort.map((field) => ({ field, order: 'asc' })), + } as unknown as ListViewSchema; + render( + + + + + , + ); +} + +function sortPanel(): HTMLElement { + const dialog = screen.getByText('Sort Records').closest('[role="dialog"]'); + if (!dialog) throw new Error('no popover content around "Sort Records"'); + return dialog as HTMLElement; +} + +/** The field triggers of the sort rows: each row is `[field select, direction select]`. */ +function sortRowTriggers(): HTMLElement[] { + return within(sortPanel()) + .getAllByRole('combobox') + .filter((_, i) => i % 2 === 0); +} + +type Option = { label: string; disabled: boolean }; + +/** What a row's field dropdown offers, and which entries it marks unavailable. */ +async function optionsOf(trigger: HTMLElement): Promise { + fireEvent.click(trigger); + const listbox = await screen.findByRole('listbox'); + const options = within(listbox) + .getAllByRole('option') + .map((o) => ({ + label: o.textContent?.trim() ?? '', + disabled: o.getAttribute('aria-disabled') === 'true' && o.hasAttribute('data-disabled'), + })); + fireEvent.keyDown(listbox, { key: 'Escape' }); + return options; +} + +/** Open the popover once the object definition built the list (`Priority` shows it did). */ +async function openSort(): Promise { + fireEvent.click(await screen.findByRole('button', { name: /^sort/i })); + await screen.findByText('Sort Records'); + let options: Option[] = []; + await waitFor(async () => { + options = await optionsOf(sortRowTriggers()[0]); + expect(options.map((o) => o.label)).toContain('Priority'); + }); + return options; +} + +function chooseIn(trigger: HTMLElement, label: string) { + fireEvent.click(trigger); + const listbox = screen.getByRole('listbox'); + const target = within(listbox).getAllByRole('option').find((o) => o.textContent?.trim() === label); + if (!target) throw new Error(`no option "${label}"`); + fireEvent.click(target); + if (screen.queryByRole('listbox')) fireEvent.keyDown(screen.getByRole('listbox'), { key: 'Escape' }); +} + +afterEach(() => { + cleanup(); +}); + +const CASES = [ + { name: 'unreadable', field: 'secret_note', label: 'Secret Note' }, + { name: 'platform-refused', field: 'remote_status', label: 'Remote Status' }, + { name: 'relational', field: 'owner', label: 'Owner (by ID)' }, +] as const; + +describe('a field the Sort picker keeps only for the current sort is removable only (objectui#11943)', () => { + for (const c of CASES) { + it(`${c.name}: listed disabled, its row shows it, "Add sort" skips it, and removing the row drops it`, async () => { + mount({ first: c.field, sort: [c.field] }); + const options = await openSort(); + + // Listed (first, as declared) and marked unavailable; nothing else is. + expect(options).toEqual([ + { label: c.label, disabled: true }, + { label: 'Title', disabled: false }, + { label: 'Priority', disabled: false }, + ]); + // Its own row is not blank. + expect(sortRowTriggers()[0]).toHaveTextContent(c.label); + + // "Add sort" seeds the first field the user may choose, not this one. + fireEvent.click(screen.getByRole('button', { name: /add sort/i })); + await waitFor(() => expect(sortRowTriggers()).toHaveLength(2)); + expect(sortRowTriggers()[1]).toHaveTextContent('Title'); + + // The new row offers it only as unavailable, and choosing it does nothing. + expect((await optionsOf(sortRowTriggers()[1])).find((o) => o.label === c.label)).toEqual({ + label: c.label, + disabled: true, + }); + chooseIn(sortRowTriggers()[1], c.label); + expect(sortRowTriggers()[1]).toHaveTextContent('Title'); + + // Removing its row ends the exception: the field leaves the list. + const firstRow = within(sortPanel()).getByText('Sort by').parentElement as HTMLElement; + fireEvent.click(within(firstRow).getByRole('button')); + await waitFor(() => expect(sortRowTriggers()).toHaveLength(1)); + expect(sortRowTriggers()[0]).toHaveTextContent('Title'); + expect((await optionsOf(sortRowTriggers()[0])).map((o) => o.label)).toEqual(['Title', 'Priority']); + }); + } + + it('control: a field the picker lists anyway carries no flag, whether or not the sort names it', async () => { + mount({ first: 'title', sort: ['title'] }); + expect(await openSort()).toEqual([ + { label: 'Title', disabled: false }, + { label: 'Priority', disabled: false }, + ]); + fireEvent.click(screen.getByRole('button', { name: /add sort/i })); + await waitFor(() => expect(sortRowTriggers()).toHaveLength(2)); + // With nothing flagged, "Add sort" seeds the first entry, as it always has. + expect(sortRowTriggers()[1]).toHaveTextContent('Title'); + }); + + it('control: with full read and no served projection, the unreadable and refused fields are ordinary options', async () => { + mount({ first: 'secret_note', sort: ['secret_note', 'remote_status'], perms: FULL_READ, projection: false }); + const options = await openSort(); + expect(options.find((o) => o.label === 'Secret Note')).toEqual({ label: 'Secret Note', disabled: false }); + expect(options.find((o) => o.label === 'Remote Status')).toEqual({ label: 'Remote Status', disabled: false }); + }); +}); From 2b80540b7675b3563b0ad1c749a3c19348bed71a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:57:01 +0000 Subject: [PATCH 2/2] test(components,plugin-list): close the listbox before reading a row; restate the objectui#11962 in-use pin (objectui#11943) The click case leaves the dropdown open when nothing is chosen, which hides the rows from the accessibility tree; it now asserts the list is still open and closes it. The objectui#11962 in-use pin keeps its assertions; its title and comment stop describing the half-state this change ends. Adds the changeset. Claude-Session: https://claude.ai/code/session_01DBZ9bntPZ7VKyQNtJeNsgw Co-authored-by: Claude --- .changeset/11943-sort-in-use-removable-only.md | 10 ++++++++++ .../sort-builder-disabled-entry-11943.test.tsx | 2 ++ .../__tests__/ListView.sortFieldRead-11943.test.tsx | 11 +++++------ 3 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 .changeset/11943-sort-in-use-removable-only.md diff --git a/.changeset/11943-sort-in-use-removable-only.md b/.changeset/11943-sort-in-use-removable-only.md new file mode 100644 index 0000000000..a7cabd7ddf --- /dev/null +++ b/.changeset/11943-sort-in-use-removable-only.md @@ -0,0 +1,10 @@ +--- +'@object-ui/components': minor +'@object-ui/plugin-list': patch +--- + +The list's Sort picker lists a field it keeps only for the current sort as removable, never as a new choice (objectui#11943). + +`SortBuilder`'s `fields` entries take an optional `disabled`. A disabled entry is drawn as an unavailable option in every row's dropdown and cannot be chosen by click or keyboard. A row whose field it already is still shows its label, and can be changed to another field or removed. "Add sort" seeds the first entry that is not disabled, and is disabled when every entry is. An entry without the flag behaves as before. + +`ListView` sets the flag on each field its Sort picker keeps only because the current sort names it: a field the user may not read, a field the platform refuses to order by, and a relational field listed as ordering by ID. Before this, a stored or URL sort on such a field left it choosable in the picker's other rows, and "Add sort" seeded it when it came first. diff --git a/packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx b/packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx index 363d2ebbf4..2f5db5fc87 100644 --- a/packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx +++ b/packages/components/src/__tests__/sort-builder-disabled-entry-11943.test.tsx @@ -111,6 +111,8 @@ describe('SortBuilder honours `disabled` on a fields entry (objectui#11943)', () fireEvent.pointerDown(secret, { pointerType: 'mouse' }); fireEvent.pointerUp(secret, { pointerType: 'mouse' }); expect(onChange).not.toHaveBeenCalled(); + // Nothing was chosen, so the list is still open; close it to read the row. + fireEvent.keyDown(screen.getByRole('listbox'), { key: 'Escape' }); expect(fieldTriggers()[0]).toHaveTextContent('Title'); cleanup(); diff --git a/packages/plugin-list/src/__tests__/ListView.sortFieldRead-11943.test.tsx b/packages/plugin-list/src/__tests__/ListView.sortFieldRead-11943.test.tsx index 526b4025a7..d2be2d4c67 100644 --- a/packages/plugin-list/src/__tests__/ListView.sortFieldRead-11943.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.sortFieldRead-11943.test.tsx @@ -197,13 +197,12 @@ describe('the Sort picker field list asks the column read check (objectui#11943) }); }); - it('a field the current sort already names stays listed (the known half-state)', async () => { + it('a field the current sort already names stays listed, so its row is named', async () => { // objectui#11943's ruling keeps such a field listed so its row is not - // blank and can be removed. This pins the half shipped here: it stays - // listed exactly as before, with no mark, and it is still choosable in the - // picker's other rows. Listing it as removable only (marked unavailable, - // never offered as a new choice) needs a `SortBuilder` change and is - // objectui#11943's follow-up, which will change this pin. + // blank and can be removed. It is listed as removable only: disabled, so + // no other row and no "Add sort" can choose it. That half is pinned, for + // each reason the picker keeps a field, in + // `ListView.sortRemovableOnly-11943.test.tsx`. mount({ perms: RESTRICTED, sort: [{ field: 'secret_note', order: 'asc' }] }); const labels = await openSortOptions(); expect(labels).toContain('Secret Note');