mirror of
https://github.com/Hestia-Homes/assessment-model.git
synced 2026-08-02 12:58:24 +00:00
feat(ara-projects): rename the stages permission and explain "mixed" (#413)
Rename: "Update stages" → "Update stages of workstream(s)", in the table header and the add/edit dialog. (Requested as "Updaet" — corrected to "Update"; say the word if the typo was deliberate.) "Mixed" on its own raised more questions than it answered — mixed how, and what happens if I click it? It is now explained at three depths, all fed by one new pure helper (splitByPermission, unit-tested): - The chip carries the count — "Mixed · 2 of 3" — needing no interaction. - Its tooltip names which workstreams allow the permission and which don't, and states what the switch will do: grant to all N, or revoke from all N. - The switch's accessible name says the same in a sentence, so a screen reader never depends on hover. role="switch" admits no aria-checked="mixed" (ARIA 1.2), which is the other reason it has to be in the name. The edit dialog gets the same treatment: rather than a bare "permissions differ" note, it names each split before the user overwrites it. The tooltip states plainly that saving clears the split, because no surface in v1 writes divergent flags — per ADR-0021 they are readable, not authorable here. PermissionToggle now takes an explicit testId rather than deriving one from the label, so copy changes don't move the test hook.
This commit is contained in:
Daniel Roth
parent
a7b1a3982b
commit
4bec5b72d2
5 changed files with 286 additions and 32 deletions
|
|
@ -8,6 +8,7 @@ import {
|
|||
groupByContractor,
|
||||
listNames,
|
||||
permissionState,
|
||||
splitByPermission,
|
||||
type ContractorAssignment,
|
||||
type ContractorAssignmentRow,
|
||||
} from "./assignments";
|
||||
|
|
@ -122,6 +123,75 @@ describe("groupByContractor", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("splitByPermission", () => {
|
||||
const assignments = [
|
||||
assignment({
|
||||
id: "1",
|
||||
workstreamName: "Windows",
|
||||
updateStagesPermission: true,
|
||||
uploadDocumentsPermission: false,
|
||||
}),
|
||||
assignment({
|
||||
id: "2",
|
||||
projectWorkstreamId: "11",
|
||||
workstreamName: "Doors",
|
||||
updateStagesPermission: false,
|
||||
uploadDocumentsPermission: false,
|
||||
}),
|
||||
assignment({
|
||||
id: "3",
|
||||
projectWorkstreamId: "12",
|
||||
workstreamName: "Asbestos",
|
||||
updateStagesPermission: true,
|
||||
uploadDocumentsPermission: false,
|
||||
}),
|
||||
];
|
||||
|
||||
it("names the workstreams that have the permission and those that don't", () => {
|
||||
expect(splitByPermission(assignments, "updateStagesPermission")).toEqual({
|
||||
on: ["Asbestos", "Windows"],
|
||||
off: ["Doors"],
|
||||
});
|
||||
});
|
||||
|
||||
it("reads each permission independently", () => {
|
||||
expect(splitByPermission(assignments, "uploadDocumentsPermission")).toEqual({
|
||||
on: [],
|
||||
off: ["Asbestos", "Doors", "Windows"],
|
||||
});
|
||||
});
|
||||
|
||||
it("orders both lists by workstream name, matching the chips", () => {
|
||||
const { on } = splitByPermission(
|
||||
[
|
||||
assignment({ id: "1", workstreamName: "Windows" }),
|
||||
assignment({ id: "2", projectWorkstreamId: "11", workstreamName: "Doors" }),
|
||||
],
|
||||
"updateStagesPermission",
|
||||
);
|
||||
|
||||
expect(on).toEqual(["Doors", "Windows"]);
|
||||
});
|
||||
|
||||
it("does not mutate the assignments it was given", () => {
|
||||
const original = [
|
||||
assignment({ id: "1", workstreamName: "Windows" }),
|
||||
assignment({ id: "2", projectWorkstreamId: "11", workstreamName: "Doors" }),
|
||||
];
|
||||
|
||||
splitByPermission(original, "updateStagesPermission");
|
||||
|
||||
expect(original.map((a) => a.workstreamName)).toEqual(["Windows", "Doors"]);
|
||||
});
|
||||
|
||||
it("handles a contractor with no assignments", () => {
|
||||
expect(splitByPermission([], "updateStagesPermission")).toEqual({
|
||||
on: [],
|
||||
off: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("listNames", () => {
|
||||
it("joins with commas and a final 'and'", () => {
|
||||
expect(listNames([])).toBe("");
|
||||
|
|
|
|||
|
|
@ -125,6 +125,31 @@ export function groupByContractor(
|
|||
.sort((a, b) => a.organisationName.localeCompare(b.organisationName));
|
||||
}
|
||||
|
||||
/** Which of the two flags is being talked about. */
|
||||
export type PermissionKey =
|
||||
| "updateStagesPermission"
|
||||
| "uploadDocumentsPermission";
|
||||
|
||||
/**
|
||||
* Split a contractor's assignments by whether they carry one permission —
|
||||
* the detail behind a `"mixed"` switch, so the UI can name the workstreams
|
||||
* that have it and the ones that don't instead of just saying they differ.
|
||||
*
|
||||
* Both lists come back ordered by workstream name, matching the chips.
|
||||
*/
|
||||
export function splitByPermission(
|
||||
assignments: ContractorAssignment[],
|
||||
permission: PermissionKey,
|
||||
): { on: string[]; off: string[] } {
|
||||
const byName = [...assignments].sort((a, b) =>
|
||||
a.workstreamName.localeCompare(b.workstreamName),
|
||||
);
|
||||
return {
|
||||
on: byName.filter((a) => a[permission]).map((a) => a.workstreamName),
|
||||
off: byName.filter((a) => !a[permission]).map((a) => a.workstreamName),
|
||||
};
|
||||
}
|
||||
|
||||
/** `"Windows, Roofs and Doors"`. */
|
||||
export function listNames(names: string[]): string {
|
||||
if (names.length === 0) return "";
|
||||
|
|
|
|||
|
|
@ -14,7 +14,11 @@ import {
|
|||
import { Button } from "@/app/shadcn_components/ui/button";
|
||||
import { Input } from "@/app/shadcn_components/ui/input";
|
||||
import { Checkbox } from "@/app/shadcn_components/ui/checkbox";
|
||||
import type { ContractorGroup } from "@/app/api/projects/[projectId]/contractors/assignments";
|
||||
import {
|
||||
listNames,
|
||||
splitByPermission,
|
||||
type ContractorGroup,
|
||||
} from "@/app/api/projects/[projectId]/contractors/assignments";
|
||||
import type {
|
||||
OrganisationOption,
|
||||
ProjectWorkstreamOption,
|
||||
|
|
@ -160,6 +164,31 @@ export function ContractorFormDialog({
|
|||
const canSave =
|
||||
Boolean(organisation) && selected.length > 0 && !save.isLoading;
|
||||
|
||||
// Which permissions this contractor currently holds inconsistently, and how.
|
||||
// Named rather than merely flagged: the user is about to overwrite the split,
|
||||
// so they should see what it is before they do.
|
||||
const mixedPermissions = editing
|
||||
? (
|
||||
[
|
||||
{
|
||||
label: "Update stages of workstream(s)",
|
||||
state: editing.updateStages,
|
||||
key: "updateStagesPermission",
|
||||
},
|
||||
{
|
||||
label: "Upload docs",
|
||||
state: editing.uploadDocuments,
|
||||
key: "uploadDocumentsPermission",
|
||||
},
|
||||
] as const
|
||||
)
|
||||
.filter((permission) => permission.state === "mixed")
|
||||
.map(({ label, key }) => ({
|
||||
label,
|
||||
...splitByPermission(editing.assignments, key),
|
||||
}))
|
||||
: [];
|
||||
|
||||
return (
|
||||
<Dialog open onOpenChange={(open) => !open && onClose()}>
|
||||
<DialogContent className="max-h-[85vh] overflow-y-auto sm:max-w-lg">
|
||||
|
|
@ -295,9 +324,10 @@ export function ContractorFormDialog({
|
|||
onCheckedChange={(value) => setUpdateStages(value === true)}
|
||||
/>
|
||||
<span className="text-sm text-gray-800">
|
||||
Update stages
|
||||
Update stages of workstream(s)
|
||||
<span className="block text-xs text-gray-500">
|
||||
Move its work orders along the stage ladder.
|
||||
Move its work orders along the stage ladder, on every workstream
|
||||
ticked above.
|
||||
</span>
|
||||
</span>
|
||||
</label>
|
||||
|
|
@ -314,15 +344,29 @@ export function ContractorFormDialog({
|
|||
</span>
|
||||
</span>
|
||||
</label>
|
||||
{editing &&
|
||||
(editing.updateStages === "mixed" ||
|
||||
editing.uploadDocuments === "mixed") && (
|
||||
<p className="mt-2 rounded-lg border border-amber-200 bg-amber-50 px-3 py-2 text-xs text-amber-800">
|
||||
This contractor currently holds different permissions on
|
||||
different workstreams. Saving applies the boxes above to all of
|
||||
them.
|
||||
{editing && mixedPermissions.length > 0 && (
|
||||
<div
|
||||
className="mt-2 rounded-lg border border-amber-200 bg-amber-50 px-3 py-2 text-xs text-amber-800"
|
||||
data-testid="contractor-form-mixed-notice"
|
||||
>
|
||||
<p className="font-semibold">
|
||||
{editing.organisationName} holds these differently on different
|
||||
workstreams today:
|
||||
</p>
|
||||
)}
|
||||
<ul className="mt-1 space-y-0.5">
|
||||
{mixedPermissions.map(({ label, on, off }) => (
|
||||
<li key={label}>
|
||||
<span className="font-semibold">{label}</span> — on for{" "}
|
||||
{listNames(on)}, off for {listNames(off)}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
<p className="mt-1">
|
||||
Saving applies the boxes above to every workstream ticked, which
|
||||
clears the split.
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
</fieldset>
|
||||
|
||||
{warning && (
|
||||
|
|
|
|||
|
|
@ -7,7 +7,10 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
|||
import { ArrowLeft, ArrowRight, Info, Pencil, Trash2, UserPlus } from "lucide-react";
|
||||
import { Button } from "@/app/shadcn_components/ui/button";
|
||||
import { ConfirmDialog } from "@/app/components/ConfirmDialog";
|
||||
import type { ContractorGroup } from "@/app/api/projects/[projectId]/contractors/assignments";
|
||||
import {
|
||||
splitByPermission,
|
||||
type ContractorGroup,
|
||||
} from "@/app/api/projects/[projectId]/contractors/assignments";
|
||||
import type { ProjectWorkstreamOption } from "@/app/api/projects/[projectId]/contractors/queries";
|
||||
import { PermissionToggle } from "./PermissionToggle";
|
||||
import {
|
||||
|
|
@ -291,10 +294,10 @@ export function ContractorPermissionsTable({
|
|||
<th className="px-4 py-3 text-sm font-semibold text-gray-700">
|
||||
Assigned workstreams
|
||||
</th>
|
||||
<th className="px-4 py-3 text-center text-sm font-semibold text-gray-700">
|
||||
Update stages
|
||||
<th className="w-44 px-4 py-3 text-center text-sm font-semibold text-gray-700">
|
||||
Update stages of workstream(s)
|
||||
</th>
|
||||
<th className="px-4 py-3 text-center text-sm font-semibold text-gray-700">
|
||||
<th className="w-32 px-4 py-3 text-center text-sm font-semibold text-gray-700">
|
||||
Upload docs
|
||||
</th>
|
||||
<th className="px-4 py-3 text-right text-sm font-semibold text-gray-700">
|
||||
|
|
@ -310,6 +313,17 @@ export function ContractorPermissionsTable({
|
|||
!canManage ||
|
||||
(Boolean(busyOrganisationId) && !busy);
|
||||
|
||||
// Which workstreams hold each permission — the detail behind a
|
||||
// "mixed" switch, so it can say how it is mixed.
|
||||
const stagesSplit = splitByPermission(
|
||||
contractor.assignments,
|
||||
"updateStagesPermission",
|
||||
);
|
||||
const documentsSplit = splitByPermission(
|
||||
contractor.assignments,
|
||||
"uploadDocumentsPermission",
|
||||
);
|
||||
|
||||
return (
|
||||
<tr
|
||||
key={contractor.organisationId}
|
||||
|
|
@ -350,8 +364,11 @@ export function ContractorPermissionsTable({
|
|||
<td className="px-4 py-4 text-center">
|
||||
<PermissionToggle
|
||||
state={contractor.updateStages}
|
||||
label="Update stages"
|
||||
label="Update stages of workstream(s)"
|
||||
contractorName={contractor.organisationName}
|
||||
testId="permission-toggle-update-stages"
|
||||
workstreamsOn={stagesSplit.on}
|
||||
workstreamsOff={stagesSplit.off}
|
||||
disabled={frozen}
|
||||
pending={busy && setPermission.isLoading}
|
||||
onChange={(next) =>
|
||||
|
|
@ -369,6 +386,9 @@ export function ContractorPermissionsTable({
|
|||
state={contractor.uploadDocuments}
|
||||
label="Upload docs"
|
||||
contractorName={contractor.organisationName}
|
||||
testId="permission-toggle-upload-docs"
|
||||
workstreamsOn={documentsSplit.on}
|
||||
workstreamsOff={documentsSplit.off}
|
||||
disabled={frozen}
|
||||
pending={busy && setPermission.isLoading}
|
||||
onChange={(next) =>
|
||||
|
|
|
|||
|
|
@ -1,38 +1,74 @@
|
|||
"use client";
|
||||
|
||||
import type { PermissionState } from "@/app/api/projects/[projectId]/contractors/assignments";
|
||||
import { Check, Info, Minus } from "lucide-react";
|
||||
import {
|
||||
Tooltip,
|
||||
TooltipContent,
|
||||
TooltipProvider,
|
||||
TooltipTrigger,
|
||||
} from "@/app/shadcn_components/ui/tooltip";
|
||||
import {
|
||||
listNames,
|
||||
type PermissionState,
|
||||
} from "@/app/api/projects/[projectId]/contractors/assignments";
|
||||
|
||||
/**
|
||||
* One permission switch in the permissions table (issue #413).
|
||||
*
|
||||
* The two flags live on the *assignment*, not on the organisation, so a
|
||||
* contractor delivering three workstreams holds three copies of each. When
|
||||
* they disagree the switch reads **Mixed** rather than picking a side —
|
||||
* turning it on then grants the permission across every workstream that
|
||||
* contractor delivers, which is what the label says it will do.
|
||||
* they disagree the switch reads **Mixed** rather than picking a side
|
||||
* (ADR-0021).
|
||||
*
|
||||
* "Mixed" on its own raises more questions than it answers — mixed how, and
|
||||
* what happens if I click it? — so it is explained three times over, at
|
||||
* increasing depth:
|
||||
*
|
||||
* 1. The chip carries the count: `Mixed · 2 of 3`. No interaction needed.
|
||||
* 2. Its tooltip names which workstreams have the permission and which do
|
||||
* not, and states what clicking the switch will do.
|
||||
* 3. The switch's own accessible name says the same thing in a sentence, so
|
||||
* a screen-reader user never depends on the hover.
|
||||
*
|
||||
* `role="switch"` only admits `aria-checked` true or false (ARIA 1.2 — unlike
|
||||
* a checkbox, a switch has no mixed value), so the mixed case is announced
|
||||
* through the accessible name instead.
|
||||
* a checkbox, a switch has no mixed value), which is the other reason the
|
||||
* mixed case has to be announced through the name.
|
||||
*/
|
||||
export function PermissionToggle({
|
||||
state,
|
||||
label,
|
||||
contractorName,
|
||||
testId,
|
||||
workstreamsOn,
|
||||
workstreamsOff,
|
||||
disabled,
|
||||
pending,
|
||||
onChange,
|
||||
}: {
|
||||
state: PermissionState;
|
||||
/** What the permission is, e.g. "Update stages". */
|
||||
/** What the permission is, e.g. "Update stages of workstream(s)". */
|
||||
label: string;
|
||||
contractorName: string;
|
||||
/** Stable hook for tests — not derived from `label`, which is copy. */
|
||||
testId: string;
|
||||
/** Workstream names holding this permission, ordered as the chips are. */
|
||||
workstreamsOn: string[];
|
||||
/** Workstream names without it. */
|
||||
workstreamsOff: string[];
|
||||
disabled: boolean;
|
||||
pending: boolean;
|
||||
onChange: (next: boolean) => void;
|
||||
}) {
|
||||
const on = state === "on";
|
||||
const mixed = state === "mixed";
|
||||
const total = workstreamsOn.length + workstreamsOff.length;
|
||||
const plural = total === 1 ? "workstream" : "workstreams";
|
||||
|
||||
const accessibleName = mixed
|
||||
? `${label} for ${contractorName} — on for ${listNames(workstreamsOn)}, off for ${listNames(
|
||||
workstreamsOff,
|
||||
)}. Activating turns it on for all ${total} ${plural}.`
|
||||
: `${label} for ${contractorName}`;
|
||||
|
||||
return (
|
||||
<span className="inline-flex flex-col items-center gap-1">
|
||||
|
|
@ -40,12 +76,8 @@ export function PermissionToggle({
|
|||
type="button"
|
||||
role="switch"
|
||||
aria-checked={on}
|
||||
aria-label={
|
||||
mixed
|
||||
? `${label} for ${contractorName} — mixed across its workstreams. Turn on for all.`
|
||||
: `${label} for ${contractorName}`
|
||||
}
|
||||
data-testid={`permission-toggle-${label.toLowerCase().replace(/\s+/g, "-")}`}
|
||||
aria-label={accessibleName}
|
||||
data-testid={testId}
|
||||
data-state={state}
|
||||
disabled={disabled || pending}
|
||||
// A mixed switch resolves upwards: the user's next click means
|
||||
|
|
@ -65,10 +97,73 @@ export function PermissionToggle({
|
|||
].join(" ")}
|
||||
/>
|
||||
</button>
|
||||
|
||||
{mixed && (
|
||||
<span className="text-[10px] font-semibold uppercase tracking-wide text-amber-600">
|
||||
Mixed
|
||||
</span>
|
||||
<TooltipProvider delayDuration={150}>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<button
|
||||
type="button"
|
||||
// Focusable, so the breakdown is reachable without a pointer.
|
||||
className="inline-flex items-center gap-1 rounded-full px-1 py-0.5 text-[10px] font-semibold uppercase tracking-wide text-amber-600 hover:text-amber-700 focus:outline-none focus-visible:ring-2 focus-visible:ring-amber-500"
|
||||
aria-label={`What “mixed” means here: ${accessibleName}`}
|
||||
data-testid={`${testId}-mixed`}
|
||||
>
|
||||
<Info className="h-3 w-3" />
|
||||
Mixed · {workstreamsOn.length} of {total}
|
||||
</button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent
|
||||
side="top"
|
||||
sideOffset={8}
|
||||
className="max-w-xs overflow-hidden border-gray-200 p-0 shadow-xl"
|
||||
>
|
||||
<div className="border-b border-gray-100 bg-gray-50 px-3 py-2">
|
||||
<p className="text-xs font-bold uppercase tracking-wide text-gray-700">
|
||||
{label}
|
||||
</p>
|
||||
<p className="mt-0.5 text-[11px] text-gray-500">
|
||||
Set per workstream, and {contractorName}’s differ.
|
||||
</p>
|
||||
</div>
|
||||
|
||||
<ul className="space-y-1.5 px-3 py-2.5">
|
||||
{workstreamsOn.map((name) => (
|
||||
<li
|
||||
key={name}
|
||||
className="flex items-start gap-1.5 text-[11px] text-gray-700"
|
||||
>
|
||||
<Check className="mt-px h-3 w-3 shrink-0 text-emerald-600" />
|
||||
<span>
|
||||
<span className="font-semibold">{name}</span> — allowed
|
||||
</span>
|
||||
</li>
|
||||
))}
|
||||
{workstreamsOff.map((name) => (
|
||||
<li
|
||||
key={name}
|
||||
className="flex items-start gap-1.5 text-[11px] text-gray-500"
|
||||
>
|
||||
<Minus className="mt-px h-3 w-3 shrink-0 text-gray-400" />
|
||||
<span>
|
||||
<span className="font-semibold">{name}</span> — not allowed
|
||||
</span>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
|
||||
<p className="border-t border-gray-100 bg-gray-50 px-3 py-2 text-[11px] text-gray-600">
|
||||
Turning the switch on grants it on{" "}
|
||||
<span className="font-semibold">
|
||||
all {total} {plural}
|
||||
</span>
|
||||
; turning it off revokes it from all {total}. This screen always
|
||||
sets one value across everything a contractor delivers, so
|
||||
saving here clears the split.
|
||||
</p>
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</TooltipProvider>
|
||||
)}
|
||||
</span>
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue