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
37 changes: 37 additions & 0 deletions .changeset/9664-search-items-available-plural.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
---
'@object-ui/i18n': patch
'@object-ui/app-shell': patch
---

Full-page search now agrees with its own number while browsing: at exactly one
searchable item the header reads `1 item available`, not `1 items available`
(objectui#9664).

The header is one ternary with two branches. The query branch already chose its
key on `totalCount === 1` — `search.resultsCount` against
`search.resultsCountPlural`. The browse branch beside it asked for
`search.itemsAvailable` at every count, and that key had neither a plural family
nor a sibling to fall to, so `en` shipped the disagreement in the default
language; `de`, `es`, `fr` and `pt` read the same way (`1 Elemente verfügbar`,
`1 elementos disponibles`, `1 éléments disponibles`, `1 itens disponíveis`).

`search.itemsAvailableOne` is the singular half, added to all ten packs, and the
browse branch now picks between the two on `allItems.length === 1`. That is this
repo's two-key plural convention — the one `common.itemCount`/`itemCountOne` and
`detail.reactionCount`/`reactionCountOne` already use — and deliberately not an
i18next `_one`/`_other` family: full key parity across ten packs caps a family at
base plus `_one` plus `_other`, so every CLDR category a pack does not spell out
falls through to the base key, which on this key is the plural. Russian meets
that at 2 to 4 and Arabic at 2, 3 to 10 and 11 to 99. Choosing the key in the
component keeps `Intl.PluralRules` and `fallbackLng` out of the path: both halves
exist in every pack, so no count in any language can reach English.

`zh`, `ja` and `ko` carry the same string in both halves because the counter word
holds the number and there is no separate singular form; `ru` does too, because
its phrasing states no noun to agree with.

One translation changed rather than being added: `ar` wrote both numbers into one
string as a parenthesised marker. Its key can no longer be reached at one item, so
the singular half of that marker was dead weight while the parentheses still
rendered at every count the key does serve. It now uses the same noun pair that
`ar`'s `common.itemCount`/`itemCountOne` already uses.
15 changes: 14 additions & 1 deletion packages/app-shell/src/views/SearchResultsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,24 @@ export function SearchResultsPage() {
</div>

{/* Results count */}
{/*
Both branches select their key on `=== 1`, and they have to: the browse
branch used to ask for `search.itemsAvailable` at every count, so English
shipped `1 items available` at a single searchable item (objectui#9664).

The repo's two-key plural convention (`common.itemCount`/`itemCountOne`,
`detail.reactionCount`/`reactionCountOne`), NOT an i18next `_one`/`_other`
family. Key parity caps a family at base + `_one` + `_other`, so every
other CLDR category falls through to the base key — and on THIS key the
base would be the plural, which is what `ar` meets at 2, 3-10 and 11-99
and `ru` at 2-4. Picking the key here keeps `Intl.PluralRules` and
`fallbackLng` out of the path entirely.
*/}
<div className="flex items-center gap-2 text-sm text-muted-foreground">
<span>
{query.trim()
? t(totalCount === 1 ? 'search.resultsCount' : 'search.resultsCountPlural', { count: totalCount, query })
: t('search.itemsAvailable', { count: allItems.length })}
: t(allItems.length === 1 ? 'search.itemsAvailableOne' : 'search.itemsAvailable', { count: allItems.length })}
</span>
{recordsSearching && (
<span
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/**
* 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#9664 — the browse branch of the results-count ternary must pick its
* key on `=== 1`, exactly as the query branch beside it already does.
*
* Before this card the browse branch asked for `search.itemsAvailable` at every
* count, and `en` has no plural family and had no sibling key, so a viewer with
* a single searchable nav item read `1 items available` — the shipped default
* language.
*
* ⭐ This file measures the SELECTION, through a real render of the page: which
* key the component asks for, and what the `en` pack then renders. The `t` here
* resolves against the real `en` catalogue rather than an inline table, so this
* cannot go green against a copy of the English that has drifted from the pack.
* The per-pack values are `searchItemsAvailable-plural-9664.test.ts`'s, in
* `@object-ui/i18n`.
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { render, screen } from '@testing-library/react';
import React from 'react';

/** Mutable across cases: the number of searchable nav items the app exposes. */
const app = vi.hoisted(() => ({ navigation: [] as unknown[] }));
/** Every key the component asked `t` for during the last render. */
const asked = vi.hoisted(() => ({ keys: [] as string[] }));
/** The `?q=` the page is mounted with; empty means the BROWSE branch. */
const url = vi.hoisted(() => ({ search: '' }));

vi.mock('react-router-dom', () => ({
useParams: () => ({ appName: 'crm' }),
useSearchParams: () => [new URLSearchParams(url.search), vi.fn()],
Link: ({ to, children, ...rest }: any) => (
<a href={typeof to === 'string' ? to : ''} {...rest}>
{children}
</a>
),
}));

vi.mock('@object-ui/i18n', async (importOriginal) => {
const actual = await importOriginal<any>();
const at = (pack: any, dotted: string) =>
dotted.split('.').reduce((node: any, part: string) => node?.[part], pack);
return {
...actual,
useObjectTranslation: () => ({
t: (key: string, options?: Record<string, unknown>) => {
asked.keys.push(key);
// `actual.en` is the shipped catalogue, the same object the app resolves
// through `fallbackLng`. Interpolation is i18next's `{{name}}` spelling.
const value = at(actual.en, key);
if (typeof value !== 'string') return key;
return value.replace(/\{\{(\w+)\}\}/g, (_m: string, name: string) =>
String(options?.[name] ?? ''),
);
},
}),
};
});

vi.mock('@object-ui/react', async (importOriginal) => {
const actual = await importOriginal<any>();
return {
...actual,
// Browse mode queries nothing, so the count on screen is the nav count.
useRecordSearch: () => ({ results: [], isSearching: false, error: undefined }),
};
});

vi.mock('../../providers/MetadataProvider', () => ({
useMetadata: () => ({
apps: [{ name: 'crm', label: 'CRM', navigation: app.navigation }],
objects: [],
}),
}));

vi.mock('../../providers/AdapterProvider', () => ({
useAdapter: () => ({ find: vi.fn(), searchAll: vi.fn() }),
}));

vi.mock('@object-ui/auth', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
useAuth: () => ({ user: { id: 'u1' }, activeOrganization: null }),
}));

import { SearchResultsPage } from '../SearchResultsPage';

/** `n` object nav items, which is what the page counts as "searchable". */
const navItems = (n: number) =>
Array.from({ length: n }, (_, i) => ({
id: `n${i}`,
type: 'object',
objectName: `crm_object_${i}`,
label: `Object ${i}`,
}));

describe('SearchResultsPage browse count agrees with its number (objectui#9664)', () => {
beforeEach(() => {
asked.keys = [];
url.search = '';
});

it('reads "1 item available" at exactly one searchable item', () => {
app.navigation = navItems(1);
render(<SearchResultsPage />);

expect(screen.getByText('1 item available')).toBeInTheDocument();
// The defect, named as the string it shipped.
expect(screen.queryByText('1 items available')).toBeNull();
// …and the selection that produces it, so a green here cannot come from a
// pack edit that papered over a call site still asking for one key.
expect(asked.keys).toContain('search.itemsAvailableOne');
expect(asked.keys).not.toContain('search.itemsAvailable');
});

it('reads the plural at every other count, zero included', () => {
app.navigation = navItems(4);
render(<SearchResultsPage />);

expect(screen.getByText('4 items available')).toBeInTheDocument();
expect(asked.keys).toContain('search.itemsAvailable');
expect(asked.keys).not.toContain('search.itemsAvailableOne');
});

it('leaves the query branch alone — it already switched at one', () => {
// The pattern this card copied is on the adjacent line, and the card claims
// nothing about it. Measured rather than assumed, so a later edit to the
// browse branch cannot quietly take the query branch with it.
app.navigation = navItems(3);
url.search = 'q=Object 0';
render(<SearchResultsPage />);

expect(screen.getByText('1 result for "Object 0"')).toBeInTheDocument();
expect(asked.keys).toContain('search.resultsCount');
expect(asked.keys).not.toContain('search.resultsCountPlural');
// The browse branch is not evaluated at all while a query is present.
expect(asked.keys).not.toContain('search.itemsAvailable');
expect(asked.keys).not.toContain('search.itemsAvailableOne');
});
});
Loading
Loading