Skip to content
Merged
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
13 changes: 13 additions & 0 deletions .changeset/11865-o-studio-surface-shared-select.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@object-ui/app-shell': patch
---

Five of the Studio design surface's pickers use the shared `Select`, the control the rest of Studio picks with (objectui#11865, the design surface's part of that card): in the navigation editor's item inspector, *Link object*, the target picker of a page, dashboard, report, action, component or doc entry (its *Doc page* and *Book* included) and a URL entry's *Open in*; in *New object*, the record sharing (OWD) choice; and in *New automation*, the trigger under *Advanced*.

The five were browser-native selects, so they looked and behaved differently from Studio's other dropdowns. They now use the shared Radix `Select`: the same trigger, dropdown and keyboard behaviour.

What they write is unchanged. Each object, target and *Open in* choice writes the same navigation entry as before, and "Choose" still unbinds the entry by removing its target key. Each record sharing choice saves the same `sharingModel` on the new object's draft, and each trigger saves the same Start node on the new flow, "Choose later on the Start node" still saving none. Re-picking the shown option writes nothing. Each picker keeps the name its label gave the native select; *Link object* had none and has none. None of the five has a read-only state: a read-only package closes navigation editing and offers neither *New* entry, as before.

One display change: an entry whose object, or whose *Open in*, is a value the picker does not offer now shows that value, listed first. The native select showed "Choose object" or "Same tab" instead, which is not what the entry holds.

**Clause-②: no.** No published face moves: the package entry exports the same names, the surface's components take the same props, and no i18n key is added. What moves is the five controls' own markup, described above.
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,9 @@ describe('a new dashboard widget is held until it is bound (objectui#11910)', ()
await addMetric();
fireEvent.click(screen.getByTitle(/^Edit navigation/));
fireEvent.click(await screen.findByRole('button', { name: /Add nav item/ }, { timeout: 8000 }));
const picker = screen
.getAllByRole('combobox')
.find((s) => within(s).queryByRole('option', { name: new RegExp(TASK.name) }));
expect(picker, 'the new entry\'s inspector offers the object to bind it to').toBeTruthy();
fireEvent.change(picker!, { target: { value: TASK.name } });
// *Link object* is the shared `Select` since objectui#11865: open its list and pick.
fireEvent.keyDown(screen.getByTestId('nav-link-object'), { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: new RegExp(`\\(${TASK.name}\\)$`) }));
await waitFor(() => expect(mockClient.save.mock.calls.filter((c) => c[0] === 'app')).toHaveLength(1), {
timeout: 4000,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,11 @@ describe('Interfaces nav autosave — the same claim, in place of its edit gener
// Add here is bound to the fixture's object in its inspector, and the rail
// shows it by that object's name.
const addedItemsInRail = () => within(rail()).queryAllByRole('button', { name: new RegExp(OBJECT.name) }).length;
const addBoundItem = () => {
const addBoundItem = async () => {
fireEvent.click(screen.getByRole('button', { name: /Add nav item/ }));
const picker = screen.getAllByRole('combobox').find((s) => within(s).queryByRole('option', { name: new RegExp(OBJECT.name) }));
if (!picker) throw new Error('the new entry\'s inspector offers no object to bind it to');
fireEvent.change(picker, { target: { value: OBJECT.name } });
// *Link object* is the shared `Select` since objectui#11865: open its list and pick.
fireEvent.keyDown(screen.getByTestId('nav-link-object'), { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: new RegExp(`\\(${OBJECT.name}\\)$`) }));
};
const navEditingOpen = () => screen.queryAllByRole('button', { name: /Add nav item/ }).length > 0;
const savedNavEntries = () =>
Expand All @@ -400,9 +400,9 @@ describe('Interfaces nav autosave — the same claim, in place of its edit gener
await screen.findByRole('button', { name: /Add nav item/ }, { timeout: 8000 });

const release = holdNextSave();
addBoundItem();
await addBoundItem();
await screen.findByTestId('nav-autosaving', undefined, { timeout: 8000 });
addBoundItem();
await addBoundItem();
expect(addedItemsInRail()).toBe(2);
// Undo it: the buffer is back to exactly what the save in flight carries.
const rows = within(rail()).getAllByRole('button', { name: new RegExp(OBJECT.name) });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,11 +464,11 @@ describe('Studio surface — a package switch over an unsaved nav edit (objectui
fireEvent.click(screen.getByTitle(/^Edit navigation/));
await screen.findByRole('button', { name: /Add nav item/ }, SLOW);
}
const addItem = (objectName: string = TASK.name) => {
const addItem = async (objectName: string = TASK.name) => {
fireEvent.click(screen.getByRole('button', { name: /Add nav item/ }));
const picker = screen.getAllByRole('combobox').find((s) => within(s).queryByRole('option', { name: new RegExp(objectName) }));
if (!picker) throw new Error('the new entry\'s inspector offers no object to bind it to');
fireEvent.change(picker, { target: { value: objectName } });
// *Link object* is the shared `Select` since objectui#11865: open its list and pick.
fireEvent.keyDown(screen.getByTestId('nav-link-object'), { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: new RegExp(`\\(${objectName}\\)$`) }));
};
async function switchTo(name: RegExp, confirmed: boolean): Promise<void> {
fireEvent.click(await screen.findByTitle('Switch / create package', undefined, SLOW));
Expand All @@ -486,7 +486,7 @@ describe('Studio surface — a package switch over an unsaved nav edit (objectui
render(surfaceTree());
await screen.findByRole('button', { name: /Landing menu/ }, SLOW);
await openEditing();
addItem();
await addItem();
expect(addedItems()).toBe(1);

await switchTo(/Beta/, true);
Expand All @@ -501,7 +501,7 @@ describe('Studio surface — a package switch over an unsaved nav edit (objectui

// The control: an edit made after the switch saves normally, to the new package's app.
await openEditing();
addItem(BETA_ITEM.name);
await addItem(BETA_ITEM.name);
await waitFor(() => expect(server.saves.filter((s) => s.type === 'app')).toHaveLength(1), SLOW);
expect(server.saves.map((s) => [s.type, s.name, s.packageId])).toEqual([['app', 'beta_app', PKG_B]]);
expect(savedNav(server.saves[0].body)).toEqual(['Board menu', { id: 'nav_item_2' }]);
Expand All @@ -511,7 +511,7 @@ describe('Studio surface — a package switch over an unsaved nav edit (objectui
render(surfaceTree());
await screen.findByRole('button', { name: /Landing menu/ }, SLOW);
await openEditing();
addItem();
await addItem();

const release = holdLoad('app', 'beta_app');
await switchTo(/Beta/, true);
Expand Down Expand Up @@ -549,7 +549,7 @@ describe('Studio surface — a package switch over an unsaved nav edit (objectui
render(surfaceTree());
await screen.findByRole('button', { name: /Landing menu/ }, SLOW);
await openEditing();
addItem();
await addItem();

await switchTo(/Beta/, false);
expect(confirmSpy).toHaveBeenCalledTimes(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ vi.mock('./packages-io', async (importOriginal) => {
});

import { DataPillar } from './StudioDesignSurface';
import { t } from '../metadata-admin/i18n';

/**
* A model's label in the create dialog: the Settings tab's own string, read by
* its key. The picker is the shared `Select` since objectui#11865, so what it
* holds is read off its trigger and its list, by label.
*/
const modelLabel = (key: string) => t(`engine.studio.settings.${key}`, 'en');

/** Open the record-sharing picker and return the labels it lists, in order. */
async function listedModels(trigger: HTMLElement): Promise<string[]> {
fireEvent.keyDown(trigger, { key: 'ArrowDown' });
return within(await screen.findByRole('listbox'))
.getAllByRole('option')
.map((o) => o.textContent ?? '');
}

beforeEach(() => {
vi.clearAllMocks();
Expand Down Expand Up @@ -93,8 +109,8 @@ function savedBody(): Record<string, unknown> {

describe('新建对象 asks for the OWD (objectui#5418)', () => {
it('offers the baseline, defaulted to the platform’s own recommended value', async () => {
const select = (await openCreateDialog()) as HTMLSelectElement;
expect(select.value).toBe('private');
const trigger = await openCreateDialog();
expect(trigger.textContent).toBe(modelLabel('sharingPrivate'));
});

it('offers exactly the three models a brand-new object can author', async () => {
Expand All @@ -103,10 +119,13 @@ describe('新建对象 asks for the OWD (objectui#5418)', () => {
// `security-owd-unset` publish wall for the
// `security-controlled-by-parent-no-relation` one. The Settings tab keeps
// all four, where the object may since have gained a master-detail field.
const select = (await openCreateDialog()) as HTMLSelectElement;
const values = [...select.options].map((o) => o.value);
expect(values).toEqual(['private', 'public_read', 'public_read_write']);
expect(values).not.toContain('controlled_by_parent');
const labels = await listedModels(await openCreateDialog());
expect(labels).toEqual([
modelLabel('sharingPrivate'),
modelLabel('sharingPublicRead'),
modelLabel('sharingPublicReadWrite'),
]);
expect(labels).not.toContain(modelLabel('sharingControlledByParent'));
});

it('saves the accepted default as an EXPLICIT baseline on the draft', async () => {
Expand All @@ -120,8 +139,9 @@ describe('新建对象 asks for the OWD (objectui#5418)', () => {
});

it('saves a changed choice instead of the default', async () => {
const select = await openCreateDialog();
fireEvent.change(select, { target: { value: 'public_read' } });
const trigger = await openCreateDialog();
fireEvent.keyDown(trigger, { key: 'ArrowDown' });
fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: modelLabel('sharingPublicRead') }));
fillAndSubmit('Visit');
await waitFor(() => expect(mockClient.save).toHaveBeenCalled());
expect(savedBody()).toHaveProperty('sharingModel', 'public_read');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import '@testing-library/jest-dom/vitest';
import * as React from 'react';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, screen, fireEvent, cleanup, waitFor, act } from '@testing-library/react';
import { render, screen, fireEvent, cleanup, waitFor, act, within } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';
import { FlowSchema, resolveFlowTriggerKind } from '@objectstack/spec/automation';

Expand Down Expand Up @@ -115,6 +115,16 @@ async function openNew(label: string): Promise<void> {
});
}

/**
* Pick the trigger whose value is `value` under *Advanced*, by the label the
* trigger picker lists it under: the shared `Select` since objectui#11865.
*/
async function pickTrigger(value: string): Promise<void> {
const label = startFields.find((f) => f.id === 'triggerType')!.options!.find((o) => o.value === value)!.label;
fireEvent.keyDown(screen.getByRole('combobox', { name: 'Trigger' }), { key: 'ArrowDown' });
fireEvent.click(within(await screen.findByRole('listbox')).getByRole('option', { name: label }));
}

/** Name the object in the visible object picker (it commits on blur). */
async function pickObject(name: string): Promise<void> {
const object = await screen.findByRole('combobox', { name: 'Object' });
Expand Down Expand Up @@ -167,7 +177,8 @@ describe('objectui#11861 — New automation opens on starting points', () => {
// The trigger form is not on the page until Advanced is chosen.
expect(screen.queryByRole('combobox', { name: 'Trigger' })).toBeNull();
fireEvent.click(screen.getByRole('radio', { name: ADVANCED() }));
expect((screen.getByRole('combobox', { name: 'Trigger' }) as HTMLSelectElement).value).toBe('');
// Nothing chosen: the picker shows "choose later".
expect(screen.getByRole('combobox', { name: 'Trigger' }).textContent).toBe(en('engine.studio.newAutoTrigger.later'));
// Choosing a preset again folds it away.
fireEvent.click(screen.getByRole('radio', { name: en(FLOW_PRESETS[1].labelKey) }));
expect(screen.queryByRole('combobox', { name: 'Trigger' })).toBeNull();
Expand Down Expand Up @@ -218,7 +229,7 @@ describe('objectui#11861 — New automation opens on starting points', () => {
// Through Advanced, with the same trigger and object.
await openNew('From advanced');
fireEvent.click(screen.getByRole('radio', { name: ADVANCED() }));
fireEvent.change(screen.getByRole('combobox', { name: 'Trigger' }), { target: { value: preset.triggerType } });
await pickTrigger(preset.triggerType);
if (watches) await pickObject('ticket');
create();
const viaAdvanced = await savedBody(2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,11 @@ async function openEditing(): Promise<void> {
* the fixture's object in its inspector — objectui#11776: an unbound entry is
* left out of what a save sends.
*/
function addItem(): void {
async function addItem(): Promise<void> {
fireEvent.click(screen.getByRole('button', { name: /Add nav item/ }));
const picker = screen.getAllByRole('combobox').find((s) => within(s).queryByRole('option', { name: new RegExp(TASK.name) }));
if (!picker) throw new Error('the new entry\'s inspector offers no object to bind it to');
fireEvent.change(picker, { target: { value: TASK.name } });
// *Link object* is the shared `Select` since objectui#11865: open its list and pick.
fireEvent.keyDown(screen.getByTestId('nav-link-object'), { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: new RegExp(`\\(${TASK.name}\\)$`) }));
}

async function clickDone(): Promise<void> {
Expand All @@ -280,7 +280,7 @@ describe('Studio surface — "Done" sends a shown nav edit (objectui#11189)', ()
render(surfaceTree());
await treeLoaded();
await openEditing();
addItem();
await addItem();
expect(addedItemsInRail()).toBe(1);

const release = holdNextSave();
Expand Down Expand Up @@ -319,7 +319,7 @@ describe('Studio surface — "Done" sends a shown nav edit (objectui#11189)', ()
render(surfaceTree());
await treeLoaded();
await openEditing();
addItem();
await addItem();

server.fail = new Error('navigation.2: Invalid input');
await clickDone();
Expand All @@ -340,11 +340,11 @@ describe('Studio surface — an edit taken while a nav save is in flight (object
await openEditing();

const release = holdNextSave();
addItem();
await addItem();
// The autosave fires after its debounce and is held in flight.
await screen.findByTestId('nav-autosaving', undefined, { timeout: 8000 });
expect(server.appSaves).toHaveLength(1);
addItem();
await addItem();
expect(addedItemsInRail()).toBe(2);

await release();
Expand Down Expand Up @@ -373,9 +373,9 @@ describe('Studio surface — an edit taken while a nav save is in flight (object
await openEditing();

const releaseFirst = holdNextSave();
addItem();
await addItem();
await screen.findByTestId('nav-autosaving', undefined, { timeout: 8000 });
addItem();
await addItem();
await clickDone();
// A save is in flight: "Done" waits for it rather than send beside it.
expect(server.appSaves).toHaveLength(1);
Expand Down Expand Up @@ -410,7 +410,7 @@ describe('Interfaces pillar — a re-read of the same package over an unsent nav
const { rerender } = render(pillarTree(0));
await treeLoaded();
await openEditing();
addItem();
await addItem();
expect(addedItemsInRail()).toBe(1);

// Another editor's save (a page draft, the create-app flow) re-reads the app.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,9 @@ function leaveGuarded(): boolean {
async function openEditingAndAddItem(): Promise<void> {
fireEvent.click(screen.getByTitle(/^Edit navigation/));
fireEvent.click(await screen.findByRole('button', { name: /Add nav item/ }, { timeout: 8000 }));
const picker = screen.getAllByRole('combobox').find((s) => within(s).queryByRole('option', { name: new RegExp(TASK.name) }));
if (!picker) throw new Error('the new entry\'s inspector offers no object to bind it to');
fireEvent.change(picker, { target: { value: TASK.name } });
// *Link object* is the shared `Select` since objectui#11865: open its list and pick.
fireEvent.keyDown(screen.getByTestId('nav-link-object'), { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: new RegExp(`\\(${TASK.name}\\)$`) }));
expect(addedItemOnScreen()).toBe(true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* so the ordering is defensive, not a live path), and
* - the picker nonetheless shows the CANONICAL target, with both names
* present as options so the wrong reading would have shown the other one
* rather than an empty select.
* rather than an empty picker.
* The third case pins the write side: picking an object emits a nav item that
* the spec's own `NavigationItemSchema` accepts whole.
*/
Expand Down Expand Up @@ -79,16 +79,18 @@ describe('StudioNavItemInspector — object binding (objectui#4881)', () => {

it('binds the picker to `objectName`, not to the rejected `object`', () => {
renderInspector(BOTH_KEYS_NODE);
// Before the flip this select showed `legacy_lead` — a real option here,
// Before the flip this picker showed `legacy_lead` — a real option here,
// so this assertion fails loudly on the old reading rather than degrading
// to an empty value.
expect(screen.getByRole('combobox')).toHaveValue('crm_lead');
// to an empty value. The picker is the shared `Select` since objectui#11865:
// its trigger shows the chosen option's label.
expect(screen.getByRole('combobox').textContent).toBe('Leads (crm_lead)');
});

it('picking an object writes a nav item the spec accepts whole', () => {
it('picking an object writes a nav item the spec accepts whole', async () => {
// The entry as the canvas births it: label-less since objectui#11196.
const onNavPatch = renderInspector({ id: 'nav_lead', type: 'object' });
fireEvent.change(screen.getByRole('combobox'), { target: { value: 'crm_lead' } });
fireEvent.keyDown(screen.getByRole('combobox'), { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: 'Leads (crm_lead)' }));

expect(onNavPatch).toHaveBeenCalledTimes(1);
const patched = (onNavPatch.mock.calls[0][0] as { navigation: Array<Record<string, unknown>> })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,24 @@ describe('objectui#11196 — emptying the field restores inheritance', () => {
});
});

/** Pick an object in *Link object*, the shared `Select` since objectui#11865. */
async function pickObject(label: string): Promise<void> {
fireEvent.keyDown(screen.getByRole('combobox'), { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: label }));
}

describe('objectui#11196 — binding an object leaves the label as it is', () => {
it('a label-less entry stays label-less', () => {
it('a label-less entry stays label-less', async () => {
const onNavPatch = renderInspector({ id: 'nav_item_3', type: 'object' });
fireEvent.change(screen.getByRole('combobox'), { target: { value: 'account' } });
await pickObject('Customers (account)');
const entry = written(onNavPatch);
expect(entry).toMatchObject({ id: 'nav_item_3', type: 'object', objectName: 'account' });
expect(has(entry, 'label')).toBe(false);
});

it('a legacy stored "New item" label is a present label: it is kept, verbatim', () => {
it('a legacy stored "New item" label is a present label: it is kept, verbatim', async () => {
const onNavPatch = renderInspector({ id: 'nav_item_3', type: 'object', label: 'New item' });
fireEvent.change(screen.getByRole('combobox'), { target: { value: 'account' } });
await pickObject('Customers (account)');
expect(written(onNavPatch).label).toBe('New item');
});
});
Loading
Loading