Repository navigation
fix: pagination controls submit the parent form when nested in a dialog #117
Description
Summary
components/shared/Pagination.tsx wraps @clickhouse/click-ui's Pagination, which renders its controls as bare <button> elements with no explicit type attribute. Per the HTML spec, the missing-value default for <button type> is the Submit Button state, so wherever this component is rendered inside a <form>, activating a pagination control submits that form instead of changing page.
This affects every paginated list nested in a dialog form:
access/EditRoleDialog.tsx— members list inside<form onSubmit={handleSubmit}>access/EditGroupDialog.tsx— same structure, same defectshared/FormDialog.tsx— generic dialog wrapper, so any future paginated child inherits the behaviour
The user-visible result is that the list cannot be paged at all: the submit path runs the save mutation and closes the dialog (or bounces to the first tab when validation fails), and no request is issued for the next offset. Standalone usages (RolesTab, GroupsTab, AuditLogTab) are not affected because they are not nested in a form.
The backend pagination contract is correct and unchanged — the client simply never issues the request for the next offset.
Correction to earlier assumptions
Two things I had assumed before reading the actual code turned out to be wrong:
| What I assumed | What the code actually shows |
|---|---|
The prev/next buttons live in Pagination.tsx and need type="button" |
Pagination.tsx does not render any buttons. It is a thin wrapper around @clickhouse/click-ui's Pagination. The buttons belong to the third-party library. |
The ESLint rule valid-button-type.js enforces an explicit HTML type on buttons |
It only validates the click-ui variant enum (primary, secondary, empty, danger, ghost). It never inspects the HTML type attribute. |
The relevant precedent is commit 82abf71a — "chore: Bump Dependencies & Fix Add / Remove Item form submission":
Updated the onClick handlers in both AddItemButton and TrashButton to prevent default button behavior before executing the provided onClick function.
The project has already hit and patched exactly this class of bug — but only button by button. Pagination was never addressed. That is the angle of this PR.
Root cause
Component tree:
EditRoleDialog / EditGroupDialog
└── <form onSubmit={handleSubmit}> (entire dialog is a form)
└── Tabs.Content value="members"
└── <MemberList />
└── <Pagination /> src/components/shared/Pagination.tsx
└── <CUIPagination /> @clickhouse/click-ui
└── <button> no type attribute
HTML spec: "The missing value default and invalid value default for the type attribute of <button> are the Submit Button state."
So every click on prev / next / a page number triggers:
form submit → handleSubmit → doSubmit() → updateMutation.mutate() → onSuccess → onClose()
The dialog closes (or jumps to the details tab if the name is empty) and the page never changes.
Unlike AddItemButton / TrashButton, we cannot apply the e.preventDefault() fix here — CUIPagination only exposes onChange(page: number), never the DOM event. click-ui also exposes no prop to set the underlying button type.
Changes
Primary fix
hooks/useButtonType.ts(new): stampstype="button"onto descendant<button>elements that have no explicit type. Follows the existing convention for patching non-parameterisable click-ui output (useStripAriaExpanded, and theDatePickerCellwrapper inAuditLogTab).components/shared/Pagination.tsx: wrapsCUIPaginationin adivwith the hook ref; re-stamps on everycurrentPage/totalPageschange because click-ui re-renders its button set when either value changes.
Defense-in-depth
EditRoleDialog.tsx,EditGroupDialog.tsx,FormDialog.tsx:handleSubmitchecksSubmitEvent.submitterand only proceeds whendata-dialog-action="submit"is present on the explicit save button.
Note: this assumes click-ui's
Buttonforwards unknown props to the DOM node. Happy to drop this layer if maintainers prefer the Pagination-only fix.
Tests
Pagination.test.tsx: pagination inside a<form>does not triggeronSubmitwhen changing page.e2e/access.spec.ts(optional): open Edit role → Members → paginate → dialog stays open.
Audited, not changed
RolesTab.tsx,GroupsTab.tsx,AuditLogTab.tsx—<Pagination>rendered outside any<form>AddItemButton.tsx,TrashButton.tsx— already patched viapreventDefaultin82abf71aUserSearchInline.tsx,SelectedMemberList.tsx,EditButton.tsx,KebabMenu.tsx,StatusToggle.tsx,SearchInput.tsx— audited for the same risk inside dialog forms; no changes needed in this PR
Optional follow-up
- ESLint rule
require-explicit-button-typeundertools/eslint-plugin-click-ui/rules/
Test plan
- Open Edit role → Members tab → click next/prev/page number → dialog stays open, page changes
- Open Edit group → Members tab → same behaviour
- Click Save → dialog submits and closes as before
- Standalone pagination in Roles, Groups, Audit log tabs still works
-
Pagination.test.tsx— "does not submit the parent form when changing page" passes
Activity