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
4 changes: 2 additions & 2 deletions DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6583,8 +6583,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, Twin sets, US
aluminium, US steel), the reader's own two first; headings are `aria-hidden` labels on a
`volumeGroupsFor` keeps the rows browsable under four headings (Metric singles, US aluminium, US
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
Expand Down
22 changes: 12 additions & 10 deletions src/components/dives/volume-combobox.render.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,17 +139,17 @@ 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(<Harness />);

await user.click(field());

expect(groupNames()).toEqual([
"Metric singles",
"Twin sets",
"US aluminium",
"US steel",
"Twin sets",
]);
expect(optionNames()).toEqual([
"3 L",
Expand All @@ -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)",
Expand All @@ -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.
Expand Down Expand Up @@ -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(<Harness />);

Expand All @@ -272,7 +274,7 @@ describe("VolumeCombobox", () => {
}

expect(screen.getByRole("option", { selected: true })).toHaveTextContent(
"14 L (2x7 L)",
"5.7 L (AL40)",
);
});
});
41 changes: 23 additions & 18 deletions src/components/dives/volume-combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}

Expand Down Expand Up @@ -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" },
Expand Down Expand Up @@ -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),
];
}

Expand Down
Loading