From 6556834561897d8d15ad8b4f16617bc489c6d37d Mon Sep 17 00:00:00 2001 From: Aleksei Vesnin Date: Wed, 23 Sep 2026 08:49:58 +0300 Subject: [PATCH 1/2] feat(dives): twin sets sit below every single-cylinder group A twin set belongs to neither unit family - it is asked for by its total whichever way a diver reads - and it is the rarer pick, so the group carries no `system` and trails both families instead of riding in front of the imperial presets for a metric reader. Verified by the two render tests asserting the full preset order in each unit system, plus the arrow-key walk over the first heading. Co-Authored-By: Claude Opus 5 --- DECISIONS.md | 7 ++-- .../dives/volume-combobox.render.test.tsx | 22 +++++----- src/components/dives/volume-combobox.tsx | 41 +++++++++++-------- 3 files changed, 39 insertions(+), 31 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 9edfdf93..c932ef1e 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -6583,9 +6583,10 @@ else reads them. Every preset is offered in both unit systems; `showIn` and `volumeOptionsFor` do not exist, because a named preset serves the traveller handed an unfamiliar tank, whom a units filter hides it from. -`volumeGroupsFor` keeps the rows browsable under four headings (Metric singles, Twin sets, US -aluminium, US steel), the reader's own two first; headings are `aria-hidden` labels on a -`role="group"` wrapper with no index, so `nextActiveIndex` skips them. +`volumeGroupsFor` keeps the rows browsable under four headings (Metric singles, US aluminium, US +steel, Twin sets), the reader's own singles first. Twin sets carries no `system` and so trails both +families: it is asked for by its total either way, and it is the rarer pick. Headings are +`aria-hidden` labels on a `role="group"` wrapper with no index, so `nextActiveIndex` skips them. HP117 and LP95 are both 15.0 L, so the list is keyed by label. The mixture must not record working pressure: a preset is only a litre hint. diff --git a/src/components/dives/volume-combobox.render.test.tsx b/src/components/dives/volume-combobox.render.test.tsx index 2425e790..85df6e93 100644 --- a/src/components/dives/volume-combobox.render.test.tsx +++ b/src/components/dives/volume-combobox.render.test.tsx @@ -139,7 +139,7 @@ describe("VolumeCombobox", () => { // the groups come in. They are written out rather than derived from // `VOLUME_GROUPS` so that a preset filed in the wrong group fails here instead // of agreeing with itself. - it("offers every preset in metric, with the metric groups first", async () => { + it("offers every preset in metric, with the metric singles first", async () => { const user = userEvent.setup(); render(); @@ -147,9 +147,9 @@ describe("VolumeCombobox", () => { expect(groupNames()).toEqual([ "Metric singles", - "Twin sets", "US aluminium", "US steel", + "Twin sets", ]); expect(optionNames()).toEqual([ "3 L", @@ -160,10 +160,6 @@ describe("VolumeCombobox", () => { "15 L", "18 L", "20 L", - "14 L (2x7 L)", - "22.2 L (2x AL80)", - "24 L (2x12 L)", - "30 L (2x15 L)", "5.7 L (AL40)", "7.1 L (AL50)", "9 L (AL63)", @@ -179,11 +175,17 @@ describe("VolumeCombobox", () => { "15 L (LP95)", "17 L (LP108)", "19 L (LP121)", + "14 L (2x7 L)", + "22.2 L (2x AL80)", + "24 L (2x12 L)", + "30 L (2x15 L)", ]); }); it("offers every preset in imperial, with the US groups first", async () => { - // Same set, different order, each US row led by its name. The two 15 L steels + // Same set, each US row led by its name, and the twin sets last in both + // systems - they belong to neither family, so neither reader's own groups + // sit under them. The two 15 L steels // are the case a value-keyed list could not render: the HP117 and the LP95 // differ by a working pressure the mixture does not record, so they share a // litre figure and nothing else. @@ -261,8 +263,8 @@ describe("VolumeCombobox", () => { it("steps the arrow keys over a group heading rather than onto it", async () => { // Headings are not options and hold no index, so the ninth press has to clear - // the eight Metric singles and land on the first Twin set. A heading that took - // an index of its own would leave the highlight one row short. + // the eight Metric singles and land on the first US aluminium row. A heading + // that took an index of its own would leave the highlight one row short. const user = userEvent.setup(); render(); @@ -272,7 +274,7 @@ describe("VolumeCombobox", () => { } expect(screen.getByRole("option", { selected: true })).toHaveTextContent( - "14 L (2x7 L)", + "5.7 L (AL40)", ); }); }); diff --git a/src/components/dives/volume-combobox.tsx b/src/components/dives/volume-combobox.tsx index 33d05f0d..561ea9a1 100644 --- a/src/components/dives/volume-combobox.tsx +++ b/src/components/dives/volume-combobox.tsx @@ -29,12 +29,13 @@ export interface VolumeOptionGroup { * Whose divers this group is *for*, which decides only where it sits. * * Never whether it is shown: every group is offered to every diver, and - * `volumeGroupsFor` moves the reader's own two to the front rather than - * dropping the other two. See DECISIONS.md - a list that hides the unfamiliar - * half is a list that fails the traveller, who is the one person a named - * preset is for. + * `volumeGroupsFor` moves the reader's own to the front rather than dropping + * the rest. See DECISIONS.md - a list that hides the unfamiliar half is a list + * that fails the traveller, who is the one person a named preset is for. + * + * Absent where a group belongs to neither family, which sinks it below both. */ - system: UnitSystem; + system?: UnitSystem; options: VolumeOption[]; } @@ -75,10 +76,11 @@ export const VOLUME_GROUPS: VolumeOptionGroup[] = [ }, { label: "Twin sets", - // Filed metric because three of the four are metric compositions, and a twin - // set is asked for by its total either way. The 2x AL80 rides along rather - // than splitting the group by material. - system: "metric", + // No `system`, so this sits under both families whichever way the diver + // reads: a twin set is asked for by its total either way, and it is the + // rarer pick, so the singles a diver reaches for most are the rows nearest + // the box. The 2x AL80 rides along rather than splitting the group by + // material. options: [ { value: 14, label: "14 L (2x7 L)" }, { value: 22.2, label: "22.2 L (2x AL80)", imperialName: "2x AL80" }, @@ -138,21 +140,24 @@ export const VOLUME_GROUPS: VolumeOptionGroup[] = [ /** * The groups in the order a diver reading in `units` sees them. * - * The reader's own two lead and the other two follow; nothing is dropped. That - * is the whole of the difference, and it is deliberate that membership is not - * part of it. A preset list filtered by unit system reads sensible and fails the - * one diver it exists for: hand a metric diver an AL80 on a boat in Florida and - * the very preset that tells them it holds 11.1 L is the one that has been - * hidden, and an imperial diver handed a 12 L in Croatia is stuck the same way. - * The unfamiliar half is the half worth showing - see DECISIONS.md. + * The reader's own singles lead, the other family's follow, and the groups + * belonging to neither sit at the bottom; nothing is dropped. That is the whole + * of the difference, and it is deliberate that membership is not part of it. A + * preset list filtered by unit system reads sensible and fails the one diver it + * exists for: hand a metric diver an AL80 on a boat in Florida and the very + * preset that tells them it holds 11.1 L is the one that has been hidden, and an + * imperial diver handed a 12 L in Croatia is stuck the same way. The unfamiliar + * half is the half worth showing - see DECISIONS.md. * * Grouping is what keeps the full list browsable, which is the job the split was * wrongly doing. */ export function volumeGroupsFor(units: UnitSystem): VolumeOptionGroup[] { + const families = VOLUME_GROUPS.filter((group) => group.system !== undefined); return [ - ...VOLUME_GROUPS.filter((group) => group.system === units), - ...VOLUME_GROUPS.filter((group) => group.system !== units), + ...families.filter((group) => group.system === units), + ...families.filter((group) => group.system !== units), + ...VOLUME_GROUPS.filter((group) => group.system === undefined), ]; } From 46445898610ba7708ccd2722133fa656d15fc4b8 Mon Sep 17 00:00:00 2001 From: Aleksei Vesnin Date: Wed, 23 Sep 2026 08:56:14 +0300 Subject: [PATCH 2/2] docs: the preset headings entry records the order, not the reason The site comment carries why twin sets trail both families; the entry states the order the dropdown renders in and nothing more. Co-Authored-By: Claude Opus 5 --- DECISIONS.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index c932ef1e..3344fd2f 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -6584,9 +6584,8 @@ else reads them. Every preset is offered in both unit systems; `showIn` and `volumeOptionsFor` do not exist, because a named preset serves the traveller handed an unfamiliar tank, whom a units filter hides it from. `volumeGroupsFor` keeps the rows browsable under four headings (Metric singles, US aluminium, US -steel, Twin sets), the reader's own singles first. Twin sets carries no `system` and so trails both -families: it is asked for by its total either way, and it is the rarer pick. Headings are -`aria-hidden` labels on a `role="group"` wrapper with no index, so `nextActiveIndex` skips them. +steel, Twin sets), the reader's own singles first; headings are `aria-hidden` labels on a +`role="group"` wrapper with no index, so `nextActiveIndex` skips them. HP117 and LP95 are both 15.0 L, so the list is keyed by label. The mixture must not record working pressure: a preset is only a litre hint.