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
32 changes: 32 additions & 0 deletions .changeset/user-filters-button-type-os6952.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
---
"@object-ui/plugin-list": patch
---

`UserFilters` preset tab buttons no longer submit an enclosing form; all six buttons declare `type="button"`

An HTML `<button>` defaults to `type="submit"` inside a `<form>`, so a preset
filter tab (`filter-tab-*`, tabs mode) submitted the enclosing form on every
click. The three buttons objectstack#6952 named now declare `type="button"`
explicitly — the dropdown chip trigger (`filter-badge-*`), the overflow trigger
(`user-filters-more`) and the preset tab — joining the session-tab buttons that
objectstack#5236 already declared it on.

Only one of the three was actually at risk, and the difference is measured
rather than assumed. The chip and the overflow trigger are
`PopoverTrigger asChild` children, and Radix's `PopoverTrigger` renders
`Primitive.button type="button"`; its Slot merges that onto a child declaring no
`type` of its own, so both already rendered as `button`. Reverting the change
confirms it: those two keep reading `button`, the plain preset tab button reads
`null`. For the two triggers this therefore moves a contract out of an upstream
implementation detail and into local source — the same reasoning objectui#3344
wrote onto the Combobox trigger — while the preset tab is a real fix.

Dormant rather than live: the only mount point today is `ListView`'s toolbar,
which is not inside a form, so no shipped screen submitted anything. The new
tests pin every rendered `UserFilters` button, in both modes, so a future button
cannot land at the submit default and an upstream Radix change surfaces in this
package's tests instead of in a user's form.

The in-file comment claiming "a Radix trigger keeps the HTML default of `submit`"
is corrected in passing — it is the inaccuracy that propagated into
objectstack#6952's premise.
23 changes: 21 additions & 2 deletions packages/plugin-list/src/UserFilters.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -509,6 +509,13 @@ function DropdownFilters({ fields, objectDef, data, onFilterChange, maxVisible,
<Popover key={f.field}>
<PopoverTrigger asChild>
<button
// Inside a <form> a bare <button> defaults to type="submit", so an
// untyped trigger would submit the enclosing form on every click
// (objectui#3344). Radix's PopoverTrigger happens to supply
// type="button" via its Slot today, but that is an upstream
// implementation detail — declare the contract locally, exactly as
// the Combobox trigger does.
type="button"
data-testid={`filter-badge-${f.field}`}
className={cn(
'inline-flex items-center gap-1 h-7 px-2 text-xs transition-colors shrink-0 rounded-md',
Expand DownExpand Up@@ -670,6 +677,10 @@ function DropdownFilters({ fields, objectDef, data, onFilterChange, maxVisible,
<Popover>
<PopoverTrigger asChild>
<button
// Same as the chip trigger above: Radix supplies type="button"
// via its Slot today, but the contract is declared locally
// (objectui#3344).
type="button"
data-testid="user-filters-more"
className="inline-flex items-center gap-1 h-7 px-2 text-xs text-muted-foreground hover:text-foreground transition-colors shrink-0 rounded-md"
>
Expand DownExpand Up@@ -854,6 +865,12 @@ function TabFilters({ tabs, showAllRecords, allowAddTab, onFilterChange, classNa
return (
<button
key={tabId}
// A plain button, NOT a Radix trigger — nothing supplied a type, so
// this one really did render as type="submit" and clicking a preset
// tab inside a <form> submitted it (objectui#3344 family;
// objectstack#6952 measured it: the two triggers above already read
// `button`, this one read `null`).
type="button"
data-testid={`filter-tab-${tabId}`}
onClick={() => handleTabChange(tabId)}
className={cn(
Expand DownExpand Up@@ -911,8 +928,10 @@ function TabFilters({ tabs, showAllRecords, allowAddTab, onFilterChange, classNa
>
<PopoverTrigger asChild>
<button
// Explicit type: a Radix trigger keeps the HTML default of
// `submit`, which submits an enclosing form on click (#3344).
// Same as the chip trigger: Radix supplies type="button" via its
// Slot today, but the contract is declared locally (objectui#3344).
// (Corrected from "a Radix trigger keeps the HTML default of
// submit" — objectstack#6952 measured that it does not.)
type="button"
className="inline-flex items-center justify-center h-7 w-7 rounded-md text-muted-foreground hover:text-foreground hover:bg-muted shrink-0"
data-testid="filter-tab-add"
Expand Down
133 changes: 133 additions & 0 deletions packages/plugin-list/src/__tests__/UserFilters.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -269,3 +269,136 @@ describe('UserFilters — i18n resolver overrides an explicit author label (regr
expect(screen.getByTestId('filter-badge-project_type').textContent).toContain('项目类型');
});
});

describe('UserFilters — every button declares type="button" (objectstack#6952, objectui#3344 family)', () => {
// An HTML <button> defaults to `type="submit"` INSIDE a <form>, so an untyped
// filter control submits the enclosing form on every click — objectui#3344's
// shape, applied to the three UserFilters buttons that predated it.
//
// The three were NOT equally at risk, and the difference is measured, not
// assumed. Reverting the fix leaves the chip (`filter-badge-*`) and the
// overflow (`user-filters-more`) triggers reading `type="button"` anyway:
// both are `PopoverTrigger asChild` children, and Radix's PopoverTrigger
// renders `Primitive.button type="button"`, which its Slot merges onto a
// child that declares no `type` of its own. Only the preset tab button is a
// plain button — reverting makes it read `null`, i.e. submit. So the fix is
// one real (dormant) defect plus two contracts moved from an upstream
// implementation detail into local source, exactly the reasoning #3344 wrote
// down on the Combobox trigger.
//
// Dormant, not live: the only mount point today (ListView's toolbar) is not
// inside a form. These assertions keep it that way when composition changes,
// and they pin the Radix behaviour so an upstream change surfaces here rather
// than in a user's form.
const noopSubmit = (e: React.FormEvent) => e.preventDefault();

const tabsConfig = {
element: 'tabs' as const,
tabs: [
{ name: 'all', label: 'All', isDefault: true },
{ name: 'urgent', label: 'Urgent', filter: [{ field: 'priority', operator: 'equals', value: 'urgent' }] },
],
};

// Radix-supplied today; the assertion pins the rendered contract either way.
it('dropdown chip trigger declares type="button" and does not submit an enclosing form', () => {
const onSubmit = vi.fn(noopSubmit);
render(
<form onSubmit={onSubmit}>
<UserFilters
config={{ element: 'dropdown', fields: [{ field: 'status' }] }}
objectDef={objectDef}
data={[]}
onFilterChange={() => {}}
/>
</form>,
);

const chip = screen.getByTestId('filter-badge-status');
expect(chip.getAttribute('type')).toBe('button');
fireEvent.click(chip);
expect(onSubmit).not.toHaveBeenCalled();
});

it('the chip clear affordance does not submit the enclosing form either', () => {
// The × lives INSIDE the chip button and only stopPropagation()s, which
// does not cancel a submit button's activation behaviour — so the chip's
// own `type` is what keeps a clear click from submitting.
const onSubmit = vi.fn(noopSubmit);
render(
<form onSubmit={onSubmit}>
<UserFilters
config={{ element: 'dropdown', fields: [{ field: 'status' }] }}
objectDef={objectDef}
data={[]}
onFilterChange={() => {}}
initialSelections={{ status: ['todo'] }}
/>
</form>,
);

fireEvent.click(screen.getByTestId('filter-clear-status'));
expect(onSubmit).not.toHaveBeenCalled();
});

// Radix-supplied today, like the chip.
it('the "More" overflow trigger declares type="button" and does not submit an enclosing form', () => {
const onSubmit = vi.fn(noopSubmit);
render(
<form onSubmit={onSubmit}>
<UserFilters
config={{ element: 'dropdown', fields: [{ field: 'status' }, { field: 'points' }] }}
objectDef={objectDef}
data={[]}
onFilterChange={() => {}}
maxVisible={1}
/>
</form>,
);

const more = screen.getByTestId('user-filters-more');
expect(more.getAttribute('type')).toBe('button');
fireEvent.click(more);
expect(onSubmit).not.toHaveBeenCalled();
});

// The one that genuinely rendered as submit before this change.
it('preset tab buttons declare type="button" and do not submit an enclosing form', () => {
const onSubmit = vi.fn(noopSubmit);
render(
<form onSubmit={onSubmit}>
<UserFilters config={tabsConfig} objectDef={objectDef} data={[]} onFilterChange={() => {}} />
</form>,
);

const preset = screen.getByTestId('filter-tab-urgent');
expect(preset.getAttribute('type')).toBe('button');
expect(screen.getByTestId('filter-tab-all').getAttribute('type')).toBe('button');
fireEvent.click(preset);
expect(onSubmit).not.toHaveBeenCalled();
});

it('no rendered UserFilters button is left at the submit default (sweep, both modes)', () => {
// A sweep rather than three named checks: a future button added to this
// file is caught here without anyone remembering to extend the list.
const { container: dropdownContainer } = render(
<UserFilters
config={{ element: 'dropdown', fields: [{ field: 'status' }, { field: 'points' }] }}
objectDef={objectDef}
data={[]}
onFilterChange={() => {}}
maxVisible={1}
/>,
);
const dropdownButtons = Array.from(dropdownContainer.querySelectorAll('button'));
expect(dropdownButtons.length).toBeGreaterThan(0);
expect(dropdownButtons.map(b => b.getAttribute('type'))).toEqual(dropdownButtons.map(() => 'button'));

const { container: tabsContainer } = render(
<UserFilters config={tabsConfig} objectDef={objectDef} data={[]} onFilterChange={() => {}} />,
);
const tabButtons = Array.from(tabsContainer.querySelectorAll('button'));
expect(tabButtons.length).toBeGreaterThan(0);
expect(tabButtons.map(b => b.getAttribute('type'))).toEqual(tabButtons.map(() => 'button'));
});
});
Loading