diff --git a/.changeset/29074-phone-list-flow.md b/.changeset/29074-phone-list-flow.md new file mode 100644 index 00000000000..76ac866a17b --- /dev/null +++ b/.changeset/29074-phone-list-flow.md @@ -0,0 +1,24 @@ +--- +'@object-ui/plugin-list': patch +'@object-ui/plugin-grid': patch +'@object-ui/components': patch +'@object-ui/app-shell': patch +'@object-ui/mobile': patch +--- + +fix(ui): the list flow works at phone width — 44 px touch targets, panels inset from the screen edge, a pager on the card list, pull-to-refresh only at the top, and an active sort shown on the bar + +Clause-②: no +Metadata-protocol: none + +Measured in a real browser at 375 and 390 px on the CRM and showcase examples. Every change below applies below the `sm` breakpoint only, so wider screens are unchanged, with two exceptions said where they occur. + +- **Touch targets.** The list bar's view switcher, Filter, Sort, Display and phone search (its field and Clear too) were 24 to 32 px. They are 44 px now. The same goes for the view switcher's menu (search, rows, star, pin, New view, Manage views), the Filter panel (field, operator, value, Remove, Add filter, Clear all, And/Or), the Sort builder, the Group editor and the Display panel's rows, switches and footer buttons. On a touch screen a view row's star and pin always show, because there is no hover to reveal them. This holds at every width on a touch screen, and a mouse still reveals them on hover. +- **Panels.** The Filter, Sort and Display panels keep 16 px from both screen edges. They used to run flush against one edge. +- **Whole labels.** A sort level and a filter condition give the field select its row's whole width, so a label such as "Last Modified At" is no longer cut to "Creat…" or "Accou…". The filter value field uses 16 px text, so iOS Safari does not zoom in when it is focused. +- **Long lists.** Under server pagination, the phone's card list showed only the first page (50 of 403 records), with no way to reach the rest except search or filter. It now ends in a pager (First, Previous, page N of M, Next, Last) that turns the same page the table's pager does. It uses the table's existing words. +- **Pull-to-refresh.** The list bound the gesture to its root, which never scrolls, so a downward drag halfway down the list (a reader scrolling back up) showed "Pull to refresh" and refetched. `usePullToRefresh` now starts a pull only when every scroll box between the touched node and the bound element is at its top. This applies at every width, wherever the hook is used. +- **Back from a record a list opened.** On a phone a list row pushes the record's page and now marks the push as a related row does, so the page's 「‹」 goes back in the history to the list. Back after it then leaves the list instead of opening the record again. The 「‹」 still reads the object's plural. A phone answering a drawer link replaces the list and stays unmarked; desktop is unchanged. +- **Active sort.** Below `sm` the Sort builder lives in the Display panel, so the bar used to show nothing once a list was sorted. While a sort is active, the bar now keeps the Sort trigger with its level count, which opens the builder. It steps aside while the phone search field is open. + +Nothing a view declares changes, and no public prop, package export or translation key is added. diff --git a/ui/packages/app-shell/src/views/RecordDetailView.tsx b/ui/packages/app-shell/src/views/RecordDetailView.tsx index cd273d1e20d..14a5894605f 100644 --- a/ui/packages/app-shell/src/views/RecordDetailView.tsx +++ b/ui/packages/app-shell/src/views/RecordDetailView.tsx @@ -4072,7 +4072,11 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri const parent = pageTrail.length > 0 ? pageTrail[pageTrail.length - 1] : undefined; const label = parent ? parent.t || shortRecordId(parent.i) - : phonePushed || (originFromState?.pathname && !originIsObjectList) + // A page a list pushed (objectstack#29074) still reads the object's + // plural, as Target 3 says; a page pushed from a record, of this + // object or another, reads that record's label. + : (phonePushed && !(originIsObjectList && !originPathOnly!.startsWith(`${objectListPath}/record/`))) + || (originFromState?.pathname && !originIsObjectList) ? originFromState?.label ?? plural : plural; const face = ( diff --git a/ui/packages/app-shell/src/views/objectViewPluginSchema.ts b/ui/packages/app-shell/src/views/objectViewPluginSchema.ts index 576a07f547e..e63fffdd6a6 100644 --- a/ui/packages/app-shell/src/views/objectViewPluginSchema.ts +++ b/ui/packages/app-shell/src/views/objectViewPluginSchema.ts @@ -12,6 +12,7 @@ import { normalizeListViewSchema } from '@object-ui/core'; import type { useObjectLabel, useObjectTranslation } from '@object-ui/i18n'; import type { useServedViewItems } from '../hooks/useServedViewItems.js'; import { viewTabLabel } from './viewTabLabel.js'; +import { RECORD_PAGE_PUSHED_STATE } from './recordPagePush.js'; import type { buildViewTabs } from './objectViewTabs.js'; import type { ConsoleObjectViewProps } from './ObjectView.js'; @@ -104,6 +105,10 @@ export function useObjectViewPluginSchema({ pathname: pathname + (search || ''), label: viewTabLabel({ id: activeView?.id, name: activeView?.name, label: activeView?.label }, objectDef.name, servedViews, viewLabel, unlabelledViewText, language), }, + // The page is pushed over this list, so on a phone its 「‹」 + // goes back in the history to it (objectstack#29074). The + // record page reads the marker on a phone only. + [RECORD_PAGE_PUSHED_STATE]: true, }; if (viewId) { navigate(`../../record/${encodeURIComponent(String(recordId))}`, { relative: 'path', state: originState }); diff --git a/ui/packages/app-shell/src/views/objectViewRecordNavigation.phonePush-29074.test.tsx b/ui/packages/app-shell/src/views/objectViewRecordNavigation.phonePush-29074.test.tsx new file mode 100644 index 00000000000..c9762d45516 --- /dev/null +++ b/ui/packages/app-shell/src/views/objectViewRecordNavigation.phonePush-29074.test.tsx @@ -0,0 +1,260 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#29074 — on a phone a list pushes a record page over itself, and + * marks the push as a related row does (`recordPagePushState`). The record + * page reads that marker on a phone (`isRecordPagePushed`) and its 「‹」 goes + * back in the history, so Back after it pops cleanly instead of leaving a + * second copy of the list on the stack. + * + * The controls: a desktop list's state is unchanged, and a phone's answer to + * a drawer link (`?recordId=`), which replaces the list's entry, is not + * marked, since there is no list below it to go back to. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { act, renderHook, render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { MemoryRouter, Routes, Route, useLocation, useNavigate, type Location } from 'react-router-dom'; +import { MetadataCtx } from '@object-ui/react'; +import { RelatedCountStore } from '@object-ui/components'; + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada', image: null }, activeOrganization: null }), + createAuthenticatedFetch: () => vi.fn(), +})); +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRecordPresence: () => [], + PresenceAvatars: () => null, +})); +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn() }), +})); +vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null })); +vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null })); +vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null })); +vi.mock('./FlowRunner', () => ({ FlowRunner: () => null })); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); + +import { useObjectViewRecordNavigation } from './objectViewRecordNavigation'; +import { useObjectViewPluginSchema } from './objectViewPluginSchema'; +import { isRecordPagePushed, recordPagePushState, RECORD_PAGE_PUSHED_STATE } from './recordPagePush'; +import { RecordDetailView } from './RecordDetailView'; + +const ROWS = [{ id: 'r-1' }, { id: 'r-2' }]; +const LIST = { pathname: '/apps/crm/crm_lead', search: '?view=all', state: null, hash: '', key: 'k' } as Location; + +function hookInputs(navigate: ReturnType, phone: boolean, search = 'view=all', appName = 'crm') { + return { + activeView: { id: 'all', name: 'all', label: 'All leads' } as never, + objectDef: { name: 'crm_lead', label: 'Lead', fields: {}, navigation: { mode: 'page' } } as never, + objects: [{ name: 'crm_lead', label: 'Lead' }] as never, + appName, + viewId: undefined, + searchParams: new URLSearchParams(search), + setSearchParams: vi.fn(), + location: { ...LIST, search: `?${search}` } as Location, + navigate, + routerRootHref: '/', + servedViews: { views: [], objects: [] } as never, + viewLabel: (_object: string, _view: string, fallback: string) => fallback, + unlabelledViewText: 'Untitled view', + language: 'en', + recordOpenPreference: 'object' as never, + phone, + }; +} + +const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} +); + +const lastState = (navigate: ReturnType) => navigate.mock.calls.at(-1)![1].state; + +describe('a list marks the record page it pushes on a phone (objectstack#29074)', () => { + it('a phone row click pushes the page with the list as its origin and the pushed marker', () => { + const navigate = vi.fn(); + const { result } = renderHook(() => useObjectViewRecordNavigation(hookInputs(navigate, true)), { wrapper }); + act(() => result.current.handleListRowsChange(ROWS)); + act(() => result.current.handleRowClick({ id: 'r-2' })); + expect(navigate.mock.calls[0][0]).toBe('record/r-2'); + expect(navigate.mock.calls[0][1].replace).toBeFalsy(); + const state = lastState(navigate); + expect(state.from).toEqual({ pathname: '/apps/crm/crm_lead?view=all', label: 'All leads', siblings: ['r-1', 'r-2'] }); + expect(isRecordPagePushed(state)).toBe(true); + }); + + it('control: a desktop row click carries no marker', () => { + const navigate = vi.fn(); + const { result } = renderHook(() => useObjectViewRecordNavigation(hookInputs(navigate, false)), { wrapper }); + act(() => result.current.handleRowClick({ id: 'r-2' })); + expect(lastState(navigate)).not.toHaveProperty(RECORD_PAGE_PUSHED_STATE); + }); + + it('control: a phone answering a drawer link replaces the list entry and carries no marker', () => { + const navigate = vi.fn(); + renderHook(() => useObjectViewRecordNavigation(hookInputs(navigate, true, 'view=all&recordId=r-1')), { wrapper }); + expect(navigate).toHaveBeenCalledTimes(1); + expect(navigate.mock.calls[0][1].replace).toBe(true); + expect(isRecordPagePushed(lastState(navigate))).toBe(false); + }); + + it('a plugin view (kanban, calendar, …) marks its row push the same way', () => { + const navigate = vi.fn(); + const { result } = renderHook(() => + useObjectViewPluginSchema({ + views: [{ id: 'all', name: 'all', label: 'All leads', type: 'kanban' }] as never, + activeViewId: 'all', + viewDraft: null, + activeView: { id: 'all', name: 'all', label: 'All leads', type: 'kanban' } as never, + objectDef: { name: 'crm_lead', label: 'Lead', fields: {} } as never, + onEdit: vi.fn(), + isAdmin: false, + location: LIST, + navigate, + viewId: undefined, + servedViews: { views: [], objects: [] } as never, + viewLabel: (_object: string, _view: string, fallback: string) => fallback, + unlabelledViewText: 'Untitled view', + language: 'en', + }), + { wrapper }, + ); + act(() => (result.current.objectViewSchema as { onNavigate: (id: string, mode: 'view') => void }).onNavigate('r-1', 'view')); + const state = lastState(navigate); + expect(state.from.pathname).toBe('/apps/crm/crm_lead?view=all'); + expect(isRecordPagePushed(state)).toBe(true); + }); +}); + +/** + * The history the marker buys, on the real record page at 390 px. Home → the + * lead list → a row → the page's 「‹」 → Back: 「‹」 returns to the list entry + * and Back leaves the list for home; the record is never reopened. The page + * still names its way back by the object's plural (Target 3, objectstack#27058). + */ +const APP = 'demo'; +const LEAD = 'crm_lead'; +const OBJECTS = [ + { + name: LEAD, + label: 'Lead', + pluralLabel: 'Leads', + fields: { id: { type: 'text', label: 'Id' }, name: { type: 'text', label: 'Name' } }, + navigation: { mode: 'page' }, + }, +]; + +function ListRoute({ phone }: { phone: boolean }) { + const navigate = useNavigate(); + const location = useLocation(); + const nav = useObjectViewRecordNavigation({ ...hookInputs(navigate, phone, 'view=all', APP), objectDef: OBJECTS[0] as never, location }); + return ; +} + +function Where() { + const location = useLocation(); + const navigate = useNavigate(); + return ( + <> + {`${location.pathname}${location.search}`} + + + ); +} + +function mountHistory(entries: Array, phone = true) { + const dataSource = { + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(async (_object: string, id: string) => ({ id, name: id === 'L-1' ? 'Mira Costa' : 'Theo Park' })), + create: vi.fn(), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + }; + const metadata = { + objects: OBJECTS, pages: [], loading: false, error: null, + refresh: async () => {}, invalidate: () => {}, ensureType: async () => [], getItem: async () => null, getItemsByType: () => [], + } as never; + render( + + + + } /> + } /> + {}} />} + /> + + + + , + ); +} + +const where = () => screen.getByTestId('where').textContent; +const backLink = () => document.querySelector('[data-record-page-root] > [data-record-top-bar] [data-record-crumb-back]') as HTMLElement | null; + +describe('the record page a phone list pushes (objectstack#29074)', () => { + beforeEach(() => { + cleanup(); + Object.defineProperty(window, 'innerWidth', { value: 390, configurable: true, writable: true }); + RelatedCountStore._reset(); + vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ data: [] }), { status: 200, headers: { 'content-type': 'application/json' } }))); + const measure = HTMLElement.prototype.getBoundingClientRect; + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) { + if (!this.hasAttribute('data-record-page-root')) return measure.call(this); + return { x: 0, y: 0, top: 0, left: 0, right: 390, bottom: 800, width: 390, height: 800, toJSON: () => ({}) } as DOMRect; + }); + }); + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + Object.defineProperty(window, 'innerWidth', { value: 1440, configurable: true, writable: true }); + }); + + it('「‹ Leads」 goes back to the list entry, and Back then leaves the list for home', async () => { + mountHistory(['/home', `/apps/${APP}/${LEAD}?view=all`]); + fireEvent.click(screen.getByTestId('row')); + await waitFor(() => expect(where()).toBe(`/apps/${APP}/${LEAD}/record/r-1`), { timeout: 5000 }); + await waitFor(() => expect(backLink()).toBeTruthy(), { timeout: 5000 }); + expect(backLink()!.tagName).toBe('BUTTON'); + expect(backLink()!.textContent).toBe('Leads'); + fireEvent.click(backLink()!); + await waitFor(() => expect(where()).toBe(`/apps/${APP}/${LEAD}?view=all`)); + fireEvent.click(screen.getByTestId('browser-back')); + await waitFor(() => expect(where()).toBe('/home')); + }); + + it('control, the push unmarked (a desktop list): 「‹」 is a link that replaces the record (#29210), so the list sits twice and Back stays on it', async () => { + mountHistory(['/home', `/apps/${APP}/${LEAD}?view=all`], false); + fireEvent.click(screen.getByTestId('row')); + await waitFor(() => expect(where()).toBe(`/apps/${APP}/${LEAD}/record/r-1`), { timeout: 5000 }); + await waitFor(() => expect(backLink()).toBeTruthy(), { timeout: 5000 }); + expect(backLink()!.tagName).toBe('A'); + expect(backLink()!.textContent).toBe('Leads'); + fireEvent.click(backLink()!); + await waitFor(() => expect(where()).toBe(`/apps/${APP}/${LEAD}?view=all`)); + // The record's entry was replaced by a second copy of the list: Back + // lands on the first copy, and only a second Back reaches home. The + // marked push above reaches home in one. + fireEvent.click(screen.getByTestId('browser-back')); + await waitFor(() => expect(screen.getByTestId('row')).toBeTruthy()); + expect(where()).toBe(`/apps/${APP}/${LEAD}?view=all`); + fireEvent.click(screen.getByTestId('browser-back')); + await waitFor(() => expect(where()).toBe('/home')); + }); + + it('a lead pushed from another lead\'s page still reads that record, not the plural', async () => { + const parent = `/apps/${APP}/${LEAD}/record/L-1`; + mountHistory([parent, { pathname: `/apps/${APP}/${LEAD}/record/r-2`, state: recordPagePushState({ pathname: parent, search: '' }, 'Mira Costa') }]); + await waitFor(() => expect(backLink()).toBeTruthy(), { timeout: 5000 }); + expect(backLink()!.tagName).toBe('BUTTON'); + expect(backLink()!.textContent).toBe('Mira Costa'); + }); +}); diff --git a/ui/packages/app-shell/src/views/objectViewRecordNavigation.ts b/ui/packages/app-shell/src/views/objectViewRecordNavigation.ts index d1f452c2171..1d167238cec 100644 --- a/ui/packages/app-shell/src/views/objectViewRecordNavigation.ts +++ b/ui/packages/app-shell/src/views/objectViewRecordNavigation.ts @@ -31,6 +31,7 @@ import type { ViewNavigationConfig } from '@object-ui/types'; import { RECORD_DRAWER_PARAM } from '../urlParams.js'; import { viewTabLabel } from './viewTabLabel.js'; import { listRecordDetailUrl, listRecordActionsValue } from './objectViewRecordLinks.js'; +import { RECORD_PAGE_PUSHED_STATE } from './recordPagePush.js'; import { expandedRecordPath, recordDrawerReturnState, @@ -238,6 +239,12 @@ export function useObjectViewRecordNavigation({ label: listLabel, ...(set ?? {}), }, + // A phone pushes the page over its list, so the page's 「‹」 + // goes back in the history to that list entry, as a page a + // related row pushes does (objectstack#29074). Not on a + // replace: the drawer link a phone answers takes the list's + // place, so there is no list entry below to go back to. + ...(phone && !replace ? { [RECORD_PAGE_PUSHED_STATE]: true } : {}), }; if (viewId) { navigate(`../../record/${encodeURIComponent(String(recordId))}`, { relative: 'path', state: originState, replace }); @@ -245,7 +252,7 @@ export function useObjectViewRecordNavigation({ navigate(`record/${encodeURIComponent(String(recordId))}`, { state: originState, replace }); } }, - [navigate, viewId, listLabel] + [navigate, viewId, listLabel, phone] ); /** * The list surface's half of the record-link mechanism (objectui#4490): diff --git a/ui/packages/app-shell/src/views/objectViewTitleSwitcher.phoneWidth-29074.test.tsx b/ui/packages/app-shell/src/views/objectViewTitleSwitcher.phoneWidth-29074.test.tsx new file mode 100644 index 00000000000..1befda33773 --- /dev/null +++ b/ui/packages/app-shell/src/views/objectViewTitleSwitcher.phoneWidth-29074.test.tsx @@ -0,0 +1,86 @@ +/** + * 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. + */ + +/** + * objectstack#29074 — the list title's view switcher at phone width. + * + * Below `sm` its trigger, the menu's search, every view row with its ☆ and 📌, + * and New view / Manage views are 44 px touch targets. From `sm` up the + * compact sizes stay. On a touch screen (no hover) a row's ☆ and 📌 always + * show; with a mouse they still appear on hover only. + * + * happy-dom applies no Tailwind, so these read the class tokens; the live + * measurement at 375 and 390 px is in the pull request's run record. + */ +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, screen, fireEvent, within } from '@testing-library/react'; +import { ObjectViewTitleSwitcher, type TitleSwitcherView } from './objectViewTitleSwitcher'; + +const t = ((key: string, options?: { defaultValue?: string }) => options?.defaultValue ?? key) as never; + +const VIEWS: TitleSwitcherView[] = [ + { id: 'all', label: 'All Records', type: 'grid', authored: true }, + { id: 'mine', label: 'My Records', type: 'grid', authored: true }, +]; + +afterEach(cleanup); + +const tokens = (el: Element) => (el.getAttribute('class') ?? '').split(/\s+/); + +function mount() { + render( + false, inMenu: () => false, toggle: vi.fn() }} + t={t} + onViewChange={vi.fn()} + onPinnedViewChange={vi.fn()} + onAddView={vi.fn()} + onManageViews={vi.fn()} + />, + ); + const trigger = screen.getByTestId('list-view-switcher'); + fireEvent.click(trigger); + return { trigger, menu: screen.getByTestId('list-view-switcher-menu') }; +} + +describe('view switcher touch targets below sm (objectstack#29074)', () => { + it('the trigger is at least 44 px below sm', () => { + const { trigger } = mount(); + expect(tokens(trigger)).toEqual(expect.arrayContaining(['max-sm:min-h-11', 'max-sm:min-w-11'])); + }); + + it('search, rows, star, pin, New view and Manage views are 44 px below sm', () => { + const { menu } = mount(); + expect(tokens(within(menu).getByTestId('list-view-switcher-search'))).toEqual(expect.arrayContaining(['h-8', 'max-sm:h-11'])); + const row = within(menu).getAllByTestId(/^view-switcher-row-/)[0]; + expect(tokens(within(row).getAllByRole('button')[0])).toEqual(expect.arrayContaining(['py-1.5', 'max-sm:min-h-11'])); + for (const id of ['view-switcher-star', 'view-switcher-pin']) { + const button = within(row).getByTestId(id); + expect(tokens(button), id).toEqual(expect.arrayContaining(['h-7', 'w-7', 'max-sm:h-11', 'max-sm:w-11'])); + } + for (const id of ['list-view-switcher-new', 'list-view-switcher-manage']) { + expect(tokens(within(menu).getByTestId(id)), id).toEqual(expect.arrayContaining(['max-sm:min-h-11'])); + } + }); + + it('an unset star and pin show on a touch screen and stay hover-only with a mouse', () => { + const { menu } = mount(); + const row = within(menu).getAllByTestId(/^view-switcher-row-/)[0]; + for (const id of ['view-switcher-star', 'view-switcher-pin']) { + expect(tokens(within(row).getByTestId(id)), id).toEqual( + expect.arrayContaining(['opacity-0', 'group-hover:opacity-100', 'pointer-coarse:opacity-100']), + ); + } + }); +}); diff --git a/ui/packages/app-shell/src/views/objectViewTitleSwitcher.tsx b/ui/packages/app-shell/src/views/objectViewTitleSwitcher.tsx index bffe4eaadf2..2f78785e509 100644 --- a/ui/packages/app-shell/src/views/objectViewTitleSwitcher.tsx +++ b/ui/packages/app-shell/src/views/objectViewTitleSwitcher.tsx @@ -154,7 +154,8 @@ export function ObjectViewTitleSwitcher({ : t('view.switcher.pin', { defaultValue: 'Pin: open this view by default' }); // Shown on hover or focus, and always while set, so a starred or pinned // view says so without a hover. - const reveal = 'opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100'; + // A touch screen has no hover, so there they always show. + const reveal = 'opacity-0 group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100 pointer-coarse:opacity-100'; return (
choose(view.id)} aria-current={active ? 'true' : undefined} className={cn( - 'flex min-w-0 flex-1 items-center gap-2 rounded-sm px-2 py-1.5 text-left text-sm', + 'flex min-w-0 flex-1 items-center gap-2 rounded-sm px-2 py-1.5 text-left text-sm max-sm:min-h-11', 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring', active ? 'font-medium text-foreground' : 'text-foreground/90', )} @@ -188,7 +189,7 @@ export function ObjectViewTitleSwitcher({ aria-disabled={inMenu || undefined} onClick={() => { if (!inMenu) favorites.toggle(view.id, view.label); }} className={cn( - 'flex h-7 w-7 shrink-0 items-center justify-center rounded-sm', + 'flex h-7 w-7 shrink-0 items-center justify-center rounded-sm max-sm:h-11 max-sm:w-11', 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring', inMenu ? 'cursor-default text-muted-foreground/40' @@ -207,7 +208,7 @@ export function ObjectViewTitleSwitcher({ aria-pressed={isPinned} onClick={() => onPinnedViewChange(isPinned ? null : view.id)} className={cn( - 'flex h-7 w-7 shrink-0 items-center justify-center rounded-sm text-muted-foreground hover:text-foreground', + 'flex h-7 w-7 shrink-0 items-center justify-center rounded-sm text-muted-foreground hover:text-foreground max-sm:h-11 max-sm:w-11', 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring', isPinned ? 'text-foreground' : reveal, )} @@ -229,7 +230,8 @@ export function ObjectViewTitleSwitcher({ // No negative margin: the title's width is its content's, and a // margin pulled out of it truncated the label it was sized for. className={cn( - 'inline-flex min-w-0 max-w-full items-center gap-1 rounded-md px-1 text-left', + // Below `sm` the trigger is a 44 px touch target. + 'inline-flex min-w-0 max-w-full items-center gap-1 rounded-md px-1 text-left max-sm:min-h-11 max-sm:min-w-11', // Inset: the title clips what overflows it (`truncate`), an outer ring included. 'hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring', open && 'bg-muted', @@ -253,7 +255,7 @@ export function ObjectViewTitleSwitcher({ placeholder={t('view.searchViews', { defaultValue: 'Search views' })} aria-label={t('view.searchViews', { defaultValue: 'Search views' })} data-testid="list-view-switcher-search" - className="h-8" + className="h-8 max-sm:h-11" />
@@ -295,7 +297,7 @@ export function ObjectViewTitleSwitcher({ type="button" data-testid="list-view-switcher-new" onClick={() => { onOpenChange(false); onAddView(); }} - className="flex w-full items-center gap-2 rounded-sm px-2 py-1.5 text-sm hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring" + className="flex w-full items-center gap-2 rounded-sm px-2 py-1.5 text-sm hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring max-sm:min-h-11" > {t('view.addNewView', { defaultValue: 'Add new view' })} @@ -306,7 +308,7 @@ export function ObjectViewTitleSwitcher({ type="button" data-testid="list-view-switcher-manage" onClick={() => { onOpenChange(false); onManageViews(); }} - className="flex w-full items-center gap-2 rounded-sm px-2 py-1.5 text-sm hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring" + className="flex w-full items-center gap-2 rounded-sm px-2 py-1.5 text-sm hover:bg-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring max-sm:min-h-11" > {t('view.manageViews', { defaultValue: 'Manage views' })} diff --git a/ui/packages/components/src/__tests__/iconOnlyButtonCensus-12143.test.ts b/ui/packages/components/src/__tests__/iconOnlyButtonCensus-12143.test.ts index 3fe55ab8387..16cad59cad4 100644 --- a/ui/packages/components/src/__tests__/iconOnlyButtonCensus-12143.test.ts +++ b/ui/packages/components/src/__tests__/iconOnlyButtonCensus-12143.test.ts @@ -272,12 +272,14 @@ describe('no shipped icon-only Button goes unnamed unless it is frozen in the ba * Since objectui#12620 `ObjectGrid.tsx` holds a SECOND such pager: a * server-grouped group's own pager, drawn in its band footer (it used to be * the per-group `data-table`'s pager), so that file reads two of each. + * Since objectstack#29074 it holds a THIRD: the phone card list's pager at + * the end of its scroller, named by the same `table.*` strings, so three. */ it('the repaired pagers are read as four named icon-only Buttons each', () => { const PAGER = ['ChevronLeft', 'ChevronRight', 'ChevronsLeft', 'ChevronsRight']; for (const [file, pagers] of [ ['packages/components/src/renderers/complex/data-table.tsx', 1], - ['packages/plugin-grid/src/ObjectGrid.tsx', 2], + ['packages/plugin-grid/src/ObjectGrid.tsx', 3], ] as const) { const chevrons = iconOnly .filter((j) => j.file === file && /^Chevrons?(Left|Right)$/.test(j.icon)) diff --git a/ui/packages/components/src/__tests__/list-builders-phone-width-29074.test.tsx b/ui/packages/components/src/__tests__/list-builders-phone-width-29074.test.tsx new file mode 100644 index 00000000000..c496cf8c7c9 --- /dev/null +++ b/ui/packages/components/src/__tests__/list-builders-phone-width-29074.test.tsx @@ -0,0 +1,109 @@ +/** + * 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. + */ + +/** + * objectstack#29074 / #29057 — the list's Sort, Filter and Group builders at + * phone width. + * + * Below `sm` every control the builders draw is a 44 px touch target, and the + * field select of a sort level or a filter condition takes its row's whole + * width, so its label is not cut to a few letters. From `sm` up the compact + * sizes and the one-line rows are unchanged: the `max-sm:` tokens sit beside + * the desktop ones, which stay. + * + * happy-dom applies no Tailwind, so these read the class tokens. A live + * measurement at 375 and 390 px is in the pull request's run record. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import React from 'react'; +import { cleanup, render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { SortBuilder } from '../custom/sort-builder'; +import { FilterBuilder } from '../custom/filter-builder'; +import { GroupingEditor } from '../custom/grouping-editor'; + +afterEach(cleanup); + +const tokens = (el: Element) => (el.getAttribute('class') ?? '').split(/\s+/); + +describe('SortBuilder below sm (objectstack#29057)', () => { + function mount() { + render( + , + ); + } + + it('lays a level out on three lines: the words, the field beside Remove, the direction', () => { + mount(); + const row = screen.getByTestId('sort-builder-row'); + expect(tokens(row)).toEqual(expect.arrayContaining(['flex', 'max-sm:grid', 'max-sm:grid-cols-[minmax(0,1fr)_auto]'])); + const field = screen.getByTestId('sort-builder-field').parentElement!; + expect(tokens(field)).toEqual(expect.arrayContaining(['flex-1', 'max-sm:row-start-2'])); + const direction = screen.getByTestId('sort-builder-direction').parentElement!; + expect(tokens(direction)).toEqual(expect.arrayContaining(['w-36', 'max-sm:col-span-2', 'max-sm:row-start-3', 'max-sm:w-auto'])); + }); + + it('every control is 44 px below sm and keeps its desktop size', () => { + mount(); + expect(tokens(screen.getByTestId('sort-builder-field'))).toEqual(expect.arrayContaining(['h-9', 'max-sm:h-11'])); + expect(tokens(screen.getByTestId('sort-builder-direction'))).toEqual(expect.arrayContaining(['h-9', 'max-sm:h-11'])); + expect(tokens(screen.getByRole('button', { name: 'Remove sort' }))).toEqual( + expect.arrayContaining(['h-9', 'w-9', 'max-sm:h-11', 'max-sm:w-11']), + ); + expect(tokens(screen.getByRole('button', { name: 'Add sort' }))).toEqual(expect.arrayContaining(['h-8', 'max-sm:h-11'])); + }); +}); + +describe('FilterBuilder below sm (objectstack#29074)', () => { + it('gives the field select the whole row and makes every control 44 px', () => { + render( + , + ); + const [field, operator] = screen.getAllByRole('combobox'); + expect(tokens(field.parentElement!)).toEqual(expect.arrayContaining(['col-span-4', 'max-sm:col-span-12'])); + expect(tokens(operator.parentElement!)).toEqual(expect.arrayContaining(['col-span-4', 'max-sm:col-span-6'])); + expect(tokens(field)).toEqual(expect.arrayContaining(['h-9', 'max-sm:h-11'])); + expect(tokens(operator)).toEqual(expect.arrayContaining(['h-9', 'max-sm:h-11'])); + const value = screen.getByPlaceholderText('Value'); + // 16 px text below sm keeps iOS Safari from zooming on focus. + expect(tokens(value)).toEqual(expect.arrayContaining(['h-9', 'max-sm:h-11', 'max-sm:text-base'])); + expect(tokens(screen.getByRole('button', { name: 'Remove condition' }))).toEqual( + expect.arrayContaining(['h-9', 'w-9', 'max-sm:h-11', 'max-sm:w-11']), + ); + expect(tokens(screen.getByRole('button', { name: /Add filter/ }))).toEqual(expect.arrayContaining(['h-8', 'max-sm:h-11'])); + expect(tokens(screen.getByRole('button', { name: /Clear all/ }))).toEqual(expect.arrayContaining(['h-7', 'max-sm:h-11'])); + }); +}); + +describe('GroupingEditor below sm (objectstack#29074)', () => { + it('makes the level controls and Add group field 44 px', () => { + render( + , + ); + expect(tokens(screen.getByTestId('grouping-field-0'))).toEqual(expect.arrayContaining(['h-7', 'max-sm:h-11'])); + for (const id of ['grouping-order-0', 'grouping-remove-0']) { + expect(tokens(screen.getByTestId(id)), id).toEqual(expect.arrayContaining(['h-7', 'w-7', 'max-sm:h-11', 'max-sm:w-11'])); + } + expect(tokens(screen.getByTestId('grouping-collapsed-0').closest('label')!)).toEqual( + expect.arrayContaining(['max-sm:h-11', 'max-sm:w-11']), + ); + expect(tokens(screen.getByTestId('grouping-add'))).toEqual(expect.arrayContaining(['h-7', 'max-sm:h-11'])); + }); +}); diff --git a/ui/packages/components/src/custom/filter-builder.tsx b/ui/packages/components/src/custom/filter-builder.tsx index 81a37609a06..333ec4cd20f 100644 --- a/ui/packages/components/src/custom/filter-builder.tsx +++ b/ui/packages/components/src/custom/filter-builder.tsx @@ -1954,7 +1954,7 @@ function FilterBuilder({ return ( - + @@ -2048,7 +2048,7 @@ function FilterBuilder({ updateCondition(condition.id, { value: value === "true" }) } > - + @@ -2113,7 +2113,8 @@ function FilterBuilder({ return ( handleValueChange(e.target.value)} @@ -2132,7 +2133,7 @@ function FilterBuilder({ variant="outline" size="sm" onClick={toggleLogic} - className="h-7 text-xs" + className="h-7 text-xs max-sm:h-11" disabled={filterGroup.logic === "and" && pairsCurrency} title={pairsCurrency ? t('filterBuilder.currencyNeedsAnd') : undefined} data-testid="filter-logic-toggle" @@ -2147,7 +2148,7 @@ function FilterBuilder({ variant="ghost" size="sm" onClick={clearAllConditions} - className="h-7 text-xs text-muted-foreground hover:text-destructive" + className="h-7 text-xs text-muted-foreground hover:text-destructive max-sm:h-11" > {t('filterBuilder.clearAll')} @@ -2164,18 +2165,21 @@ function FilterBuilder({
{visibleConditions.map((condition) => { const pairsWith = pairedCurrencyField(condition, fields, needsValueInput) - // Four controls on a paired row, three on every other. - const span = pairsWith ? "col-span-3" : "col-span-4" + // Four controls on a paired row, three on every other. Below `sm` the + // field takes the row's whole width, so its label is not cut, and the + // other controls share the line under it. + const span = pairsWith ? "col-span-3 max-sm:col-span-6" : "col-span-4 max-sm:col-span-6" + const fieldSpan = pairsWith ? "col-span-3 max-sm:col-span-12" : "col-span-4 max-sm:col-span-12" const code = pairsWith ? currencyCompanionOf(filterGroup, condition.id)?.value : undefined return (
-
+
{ const next = current.filter((_, i) => i !== idx); writeFields(next); @@ -179,7 +180,7 @@ export function GroupingEditor({