Skip to content

Commit 7fece15

Browse files
committed
fix: prevent ToggleGroup deselection in single mode
ToggleGroup now implements proper radio button behavior by default, preventing deselection and ensuring one option always remains selected. Remove redundant protection logic from individual usages. fix(settings): weight unit toggle now marks form as dirty - Add shouldDirty: true to setValue calls in handleUnitChange - Ensures Save Settings button is enabled when unit preference changes docs: clarify commit message guidelines to prevent duplicate changelog entries
1 parent 6b62bd5 commit 7fece15

6 files changed

Lines changed: 91 additions & 93 deletions

File tree

apps/web/src/components/dashboard/buttons.tsx

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,7 @@ const Buttons = () => {
2020

2121
return (
2222
<div className="flex flex-col-reverse gap-4 md:flex-row print:hidden">
23-
<ToggleGroup
24-
type="single"
25-
value={timeRange}
26-
onValueChange={(value) => {
27-
if (value && value !== "") setTimeRange(value as TimeRange);
28-
}}
29-
defaultValue="4w"
30-
aria-label="Time Range"
31-
>
23+
<ToggleGroup type="single" value={timeRange} onValueChange={(value) => setTimeRange(value as TimeRange)} defaultValue="4w" aria-label="Time Range">
3224
<ToggleGroupItem value="4w">
3325
<span className="lg:hidden">4 wk</span>
3426
<span className="hidden lg:inline">4 weeks</span>
@@ -49,15 +41,7 @@ const Buttons = () => {
4941
{!isMobile && <ToggleGroupItem value="explore">Explore</ToggleGroupItem>}
5042
</ToggleGroup>
5143

52-
<ToggleGroup
53-
type="single"
54-
value={mode}
55-
onValueChange={(value) => {
56-
if (value && value !== "") setMode(value as Mode);
57-
}}
58-
defaultValue="weight"
59-
aria-label="Mode"
60-
>
44+
<ToggleGroup type="single" value={mode} onValueChange={(value) => setMode(value as Mode)} defaultValue="weight" aria-label="Mode">
6145
<ToggleGroupItem value="weight">Weight</ToggleGroupItem>
6246
<ToggleGroupItem value="fatpercent">Fat %</ToggleGroupItem>
6347
<ToggleGroupItem value="fatmass">Fat Mass</ToggleGroupItem>

apps/web/src/components/download/view-toggle-buttons.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,7 @@ export function ViewToggleButtons({ viewType, onViewChange, providerLinks }: Vie
2020
});
2121

2222
return (
23-
<ToggleGroup
24-
type="single"
25-
value={viewType}
26-
onValueChange={(value) => {
27-
if (value && value !== "") onViewChange(value);
28-
}}
29-
defaultValue="computed"
30-
aria-label="View Type"
31-
>
23+
<ToggleGroup type="single" value={viewType} onValueChange={(value) => onViewChange(value)} defaultValue="computed" aria-label="View Type">
3224
<ToggleGroupItem value="computed">Computed Values</ToggleGroupItem>
3325
{sortedProviders.map((provider) => (
3426
<ToggleGroupItem key={provider.provider} value={provider.provider}>

apps/web/src/components/settings/settings.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,17 @@ export function Settings() {
5050
}
5151

5252
// Update the metric setting first
53-
setValue("useMetric", newIsMetric);
53+
setValue("useMetric", newIsMetric, { shouldDirty: true });
5454

5555
// Convert planned pounds per week
5656
const currentPlan = currentValues.plannedPoundsPerWeek;
5757
if (currentPlan && currentPlan !== 0) {
5858
if (newIsMetric) {
5959
// Converting from lbs to kg (roughly divide by 2)
60-
setValue("plannedPoundsPerWeek", currentPlan / 2);
60+
setValue("plannedPoundsPerWeek", currentPlan / 2, { shouldDirty: true });
6161
} else {
6262
// Converting from kg to lbs (roughly multiply by 2)
63-
setValue("plannedPoundsPerWeek", currentPlan * 2);
63+
setValue("plannedPoundsPerWeek", currentPlan * 2, { shouldDirty: true });
6464
}
6565
}
6666

@@ -70,11 +70,11 @@ export function Settings() {
7070
if (newIsMetric) {
7171
// Converting from lbs to kg (divide by 2.20462 and round)
7272
const kgValue = Math.round(currentGoalWeight / 2.20462);
73-
setValue("goalWeight", kgValue);
73+
setValue("goalWeight", kgValue, { shouldDirty: true });
7474
} else {
7575
// Converting from kg to lbs (multiply by 2.20462 and round)
7676
const lbsValue = Math.round(currentGoalWeight * 2.20462);
77-
setValue("goalWeight", lbsValue);
77+
setValue("goalWeight", lbsValue, { shouldDirty: true });
7878
}
7979
}
8080
};

apps/web/src/components/ui/toggle-group.test.tsx

Lines changed: 13 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,23 @@ import { ToggleGroup, ToggleGroupItem } from "./toggle-group";
66
describe("ToggleGroup", () => {
77
it("renders children correctly", () => {
88
render(
9-
<ToggleGroup type="single">
9+
<ToggleGroup>
1010
<ToggleGroupItem value="a">Option A</ToggleGroupItem>
1111
<ToggleGroupItem value="b">Option B</ToggleGroupItem>
1212
</ToggleGroup>,
1313
);
1414

15-
// In single mode, buttons have role="radio"
15+
// Always operates in single mode, buttons have role="radio"
1616
expect(screen.getByRole("radio", { name: "Option A" })).toBeInTheDocument();
1717
expect(screen.getByRole("radio", { name: "Option B" })).toBeInTheDocument();
1818
});
1919

20-
it("handles single selection mode", async () => {
20+
it("handles selection", async () => {
2121
const handleValueChange = vi.fn();
2222
const user = userEvent.setup();
2323

2424
render(
25-
<ToggleGroup type="single" onValueChange={handleValueChange}>
25+
<ToggleGroup onValueChange={handleValueChange}>
2626
<ToggleGroupItem value="a">Option A</ToggleGroupItem>
2727
<ToggleGroupItem value="b">Option B</ToggleGroupItem>
2828
</ToggleGroup>,
@@ -38,50 +38,27 @@ describe("ToggleGroup", () => {
3838
expect(handleValueChange).toHaveBeenCalledWith("b");
3939
});
4040

41-
it("allows deselection in single mode by default", async () => {
41+
it("prevents deselection (radio button behavior)", async () => {
4242
const handleValueChange = vi.fn();
4343
const user = userEvent.setup();
4444

4545
render(
46-
<ToggleGroup type="single" onValueChange={handleValueChange} defaultValue="a">
46+
<ToggleGroup onValueChange={handleValueChange} defaultValue="a">
4747
<ToggleGroupItem value="a">Option A</ToggleGroupItem>
4848
<ToggleGroupItem value="b">Option B</ToggleGroupItem>
4949
</ToggleGroup>,
5050
);
5151

5252
const optionA = screen.getByRole("radio", { name: "Option A" });
5353

54-
// Click the already selected option to deselect it
54+
// Click the already selected option - should not trigger onValueChange
5555
await user.click(optionA);
56-
expect(handleValueChange).toHaveBeenCalledWith("");
57-
});
58-
59-
it("handles multiple selection mode", async () => {
60-
const handleValueChange = vi.fn();
61-
const user = userEvent.setup();
62-
63-
render(
64-
<ToggleGroup type="multiple" onValueChange={handleValueChange}>
65-
<ToggleGroupItem value="a">Option A</ToggleGroupItem>
66-
<ToggleGroupItem value="b">Option B</ToggleGroupItem>
67-
<ToggleGroupItem value="c">Option C</ToggleGroupItem>
68-
</ToggleGroup>,
69-
);
70-
71-
// In multiple mode, buttons have role="button"
72-
const optionA = screen.getByRole("button", { name: "Option A" });
73-
const optionB = screen.getByRole("button", { name: "Option B" });
74-
75-
await user.click(optionA);
76-
expect(handleValueChange).toHaveBeenCalledWith(["a"]);
77-
78-
await user.click(optionB);
79-
expect(handleValueChange).toHaveBeenCalledWith(["a", "b"]);
56+
expect(handleValueChange).not.toHaveBeenCalled();
8057
});
8158

8259
it("applies correct styling to items", () => {
8360
render(
84-
<ToggleGroup type="single" defaultValue="a">
61+
<ToggleGroup defaultValue="a">
8562
<ToggleGroupItem value="a">Selected</ToggleGroupItem>
8663
<ToggleGroupItem value="b">Not Selected</ToggleGroupItem>
8764
</ToggleGroup>,
@@ -96,7 +73,7 @@ describe("ToggleGroup", () => {
9673

9774
it("applies rounded corners correctly", () => {
9875
render(
99-
<ToggleGroup type="single">
76+
<ToggleGroup>
10077
<ToggleGroupItem value="a">First</ToggleGroupItem>
10178
<ToggleGroupItem value="b">Middle</ToggleGroupItem>
10279
<ToggleGroupItem value="c">Last</ToggleGroupItem>
@@ -114,7 +91,7 @@ describe("ToggleGroup", () => {
11491

11592
it("removes duplicate borders between items", () => {
11693
render(
117-
<ToggleGroup type="single">
94+
<ToggleGroup>
11895
<ToggleGroupItem value="a">First</ToggleGroupItem>
11996
<ToggleGroupItem value="b">Second</ToggleGroupItem>
12097
</ToggleGroup>,
@@ -126,7 +103,7 @@ describe("ToggleGroup", () => {
126103

127104
it("can be disabled", () => {
128105
render(
129-
<ToggleGroup type="single" disabled>
106+
<ToggleGroup disabled>
130107
<ToggleGroupItem value="a">Option A</ToggleGroupItem>
131108
<ToggleGroupItem value="b">Option B</ToggleGroupItem>
132109
</ToggleGroup>,
@@ -140,7 +117,7 @@ describe("ToggleGroup", () => {
140117

141118
it("inherits variant and size from context", () => {
142119
render(
143-
<ToggleGroup type="single" variant="outline" size="sm">
120+
<ToggleGroup variant="outline" size="sm">
144121
<ToggleGroupItem value="a">Option A</ToggleGroupItem>
145122
</ToggleGroup>,
146123
);

apps/web/src/components/ui/toggle-group.tsx

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,43 @@ const ToggleGroupContext = React.createContext<VariantProps<typeof toggleVariant
1010
variant: "default",
1111
});
1212

13-
function ToggleGroup({
14-
className,
15-
variant = "outline",
16-
size = "default",
17-
children,
18-
...props
19-
}: React.ComponentProps<typeof ToggleGroupPrimitive.Root> & VariantProps<typeof toggleVariants>) {
13+
type ToggleGroupProps = {
14+
className?: string;
15+
variant?: VariantProps<typeof toggleVariants>["variant"];
16+
size?: VariantProps<typeof toggleVariants>["size"];
17+
children: React.ReactNode;
18+
type?: "single"; // Always single mode now
19+
onValueChange?: (value: string) => void;
20+
value?: string;
21+
defaultValue?: string;
22+
disabled?: boolean;
23+
rovingFocus?: boolean;
24+
loop?: boolean;
25+
orientation?: "horizontal" | "vertical";
26+
dir?: "ltr" | "rtl";
27+
} & Omit<React.ComponentProps<"div">, "onValueChange" | "value" | "defaultValue">;
28+
29+
function ToggleGroup({ className, variant = "outline", size = "default", children, onValueChange, value, defaultValue, ...props }: ToggleGroupProps) {
30+
const handleValueChange = React.useCallback(
31+
(value: string) => {
32+
// Prevent deselection - ignore empty values (radio button behavior)
33+
if (value && value !== "") {
34+
onValueChange?.(value);
35+
}
36+
},
37+
[onValueChange],
38+
);
39+
2040
return (
2141
<ToggleGroupPrimitive.Root
2242
data-slot="toggle-group"
2343
data-variant={variant}
2444
data-size={size}
2545
className={cn("group/toggle-group inline-flex items-center rounded-md", className)}
46+
type="single"
47+
value={value}
48+
defaultValue={defaultValue}
49+
onValueChange={handleValueChange}
2650
{...props}
2751
>
2852
<ToggleGroupContext.Provider value={{ variant, size }}>{children}</ToggleGroupContext.Provider>

docs/steering/conventions.md

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -121,34 +121,55 @@ Follow conventional commit format with these TrendWeight-specific rules:
121121
- `test:` - Adding or updating tests
122122
- `chore:` - Maintenance tasks, dependency updates
123123

124-
#### Multi-Topic Commits
125-
When a single commit spans multiple major unrelated areas, use footers to document each major topic:
124+
#### Single vs Multi-Topic Commits
125+
126+
**Single Topic Commits (Preferred)**
127+
Most commits should focus on one specific change:
128+
129+
```
130+
fix: settings form not marking dirty when weight unit changes
131+
132+
Add shouldDirty: true to setValue calls in handleUnitChange to ensure
133+
the Save Settings button is enabled when unit preference changes.
134+
```
135+
136+
**Multi-Topic Commits (Use Sparingly)**
137+
Only when you have multiple genuinely UNRELATED changes that must go together, use footers:
126138

127139
```
128-
feat: implement backend computation with UI improvements
140+
feat: add user authentication system
129141
130-
Add server-side weight trend calculations and fix several frontend issues.
142+
Implement core authentication with JWT tokens and user registration.
131143
132-
feat(api): add MeasurementComputationService for server-side trend calculations
133-
- Implement exponentially smoothed moving average (alpha=0.1)
134-
- Add ComputedMeasurement and SourceMeasurement models
135-
- Update MeasurementsResponse to use computedMeasurements array
136-
- Support optional includeSource parameter for raw data access
144+
fix(api): resolve memory leak in background sync process
145+
- Dispose HttpClient instances properly
146+
- Cancel long-running tasks on shutdown
137147
138-
refactor(web): improve progress tracking system
139-
- Centralize toast management in SyncProgressProvider to prevent duplicates
140-
- Simplify useSyncProgress API by hiding internal functions
141-
- Remove redundant useSyncProgressId hook
142-
- Fix React hooks rule violations with proper useWithProgress wrapper
148+
docs: update deployment guide with new environment variables
149+
```
150+
151+
**Critical Rules:**
152+
- **Main message describes ONE specific change**, not a summary of multiple changes
153+
- **Only use footers for additional UNRELATED changes** that deserve separate changelog entries
154+
- **Never create a footer that restates the main message** - this creates duplicate changelog entries
155+
- **If all changes are related, group them under the main message without footers**
156+
157+
**Bad Example (creates duplicate changelog entries):**
158+
```
159+
fix: improve form behavior
143160
144-
fix(web): download page skeleton width now matches actual table dimensions
161+
Fix multiple form-related issues.
145162
146-
docs: restructure documentation with steering documents and commit guidelines
163+
fix(forms): settings form not marking dirty when weight unit changes
147164
```
148165

149-
Only use footers for major topics - group related changes together rather than creating a footer for every small change. Footers can be multi-line to provide details.
166+
**Good Example:**
167+
```
168+
fix: settings form not marking dirty when weight unit changes
150169
151-
**Important**: Don't create footers that simply restate the main commit message. Only use footers when you have genuinely different types of changes that deserve separate changelog entries.
170+
Add shouldDirty: true to setValue calls in handleUnitChange to ensure
171+
the Save Settings button is enabled when unit preference changes.
172+
```
152173

153174
#### Rules
154175
- **Never use "BREAKING CHANGE"** - this is an application, not a library

0 commit comments

Comments
 (0)