fix(sheet): make "Mark done" mark it done
The button advanced one step round the untouched → doing → done cycle, so pressing "Mark done" on a fresh event produced "doing it" and needed a second press. Worse, the press after "done" cleared the status outright rather than undoing anything. Three states need three targets — that is the control directly above it. This one is the commit, so it sets done and back. cycleStatus had no other caller and is gone. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
edc6f3e4bb
commit
2a7a117705
@@ -87,6 +87,10 @@ completion the user already has.
|
|||||||
Three states, not two: untouched, doing it, done. Plus an optional effort estimate — quick, short,
|
Three states, not two: untouched, doing it, done. Plus an optional effort estimate — quick, short,
|
||||||
long, grind — and a free-text note.
|
long, grind — and a free-text note.
|
||||||
|
|
||||||
|
Three states need three targets. A single control cycling untouched → doing → done makes a button
|
||||||
|
labelled "Mark done" produce "doing it", which is the control lying about itself; the detail sheet
|
||||||
|
has an explicit control per state, and its primary action goes straight to done and back.
|
||||||
|
|
||||||
Effort is not decoration. Combined with the time remaining it answers the question the calendar
|
Effort is not decoration. Combined with the time remaining it answers the question the calendar
|
||||||
can't: *can I still finish this?* The same two days is comfortable for a quick event and hopeless
|
can't: *can I still finish this?* The same two days is comfortable for a quick event and hopeless
|
||||||
for a grind, so an event carrying an effort estimate gets a "tight" or "running out of time" flag
|
for a grind, so an event carrying an effort estimate gets a "tight" or "running out of time" flag
|
||||||
|
|||||||
@@ -118,7 +118,6 @@ export function App() {
|
|||||||
ignored.toggle(id);
|
ignored.toggle(id);
|
||||||
setLastIgnored(wasIgnored ? null : { id, title });
|
setLastIgnored(wasIgnored ? null : { id, title });
|
||||||
};
|
};
|
||||||
const toggle = prog.cycleStatus;
|
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const ac = new AbortController();
|
const ac = new AbortController();
|
||||||
@@ -424,7 +423,6 @@ export function App() {
|
|||||||
onEffort={prog.setEffort}
|
onEffort={prog.setEffort}
|
||||||
onNote={prog.setNote}
|
onNote={prog.setNote}
|
||||||
onIgnore={(id) => toggleIgnored(id, openRow.event.title)}
|
onIgnore={(id) => toggleIgnored(id, openRow.event.title)}
|
||||||
onToggle={toggle}
|
|
||||||
onClose={() => setOpenId(null)}
|
onClose={() => setOpenId(null)}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -24,7 +24,6 @@ export function EventDetail({
|
|||||||
dailyDays,
|
dailyDays,
|
||||||
onDaily,
|
onDaily,
|
||||||
onToggleDay,
|
onToggleDay,
|
||||||
onToggle,
|
|
||||||
onIgnore,
|
onIgnore,
|
||||||
onStatus,
|
onStatus,
|
||||||
onEffort,
|
onEffort,
|
||||||
@@ -47,7 +46,6 @@ export function EventDetail({
|
|||||||
dailyDays: string[];
|
dailyDays: string[];
|
||||||
onDaily: (id: string, daily: boolean | undefined) => void;
|
onDaily: (id: string, daily: boolean | undefined) => void;
|
||||||
onToggleDay: (id: string, day: string) => void;
|
onToggleDay: (id: string, day: string) => void;
|
||||||
onToggle: (id: string) => void;
|
|
||||||
onIgnore: (id: string) => void;
|
onIgnore: (id: string) => void;
|
||||||
onStatus: (id: string, s: Status | undefined) => void;
|
onStatus: (id: string, s: Status | undefined) => void;
|
||||||
onEffort: (id: string, e: Effort | undefined) => void;
|
onEffort: (id: string, e: Effort | undefined) => void;
|
||||||
@@ -198,10 +196,19 @@ export function EventDetail({
|
|||||||
</p>
|
</p>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
{/* Says what it does and does what it says.
|
||||||
|
|
||||||
|
It used to advance one step round the untouched → doing → done
|
||||||
|
cycle, so a reader pressing a button labelled "Mark done" on a fresh
|
||||||
|
event got "doing it" and had to press it again — and the second
|
||||||
|
press from "done" silently wiped the status rather than undoing
|
||||||
|
anything. Three states need three targets, which is what the control
|
||||||
|
above is; this one is the commit, so it goes straight to done and
|
||||||
|
back. */}
|
||||||
<div className="mt-5 flex items-center gap-2">
|
<div className="mt-5 flex items-center gap-2">
|
||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
onClick={() => onToggle(event.id)}
|
onClick={() => onStatus(event.id, completed ? undefined : "done")}
|
||||||
className={`flex-1 rounded-lg border px-4 py-2.5 text-sm font-medium transition-colors ${
|
className={`flex-1 rounded-lg border px-4 py-2.5 text-sm font-medium transition-colors ${
|
||||||
completed
|
completed
|
||||||
? "border-hairline text-muted hover:text-ink"
|
? "border-hairline text-muted hover:text-ink"
|
||||||
|
|||||||
@@ -91,28 +91,11 @@ export function useProgress() {
|
|||||||
[patch],
|
[patch],
|
||||||
);
|
);
|
||||||
|
|
||||||
const cycleStatus = useCallback(
|
// There was a `cycleStatus` here that advanced untouched → doing → done →
|
||||||
(id: string) => {
|
// untouched from a single control. It is gone: the only caller was the sheet's
|
||||||
setProgress((prev) => {
|
// "Mark done" button, where one press on a fresh event produced "doing it" and
|
||||||
// Untouched → doing → done → untouched. One control, three states, in
|
// the press after "done" cleared the status instead of undoing it. Status is
|
||||||
// the order the reader actually moves through them.
|
// set outright now, by a control per state.
|
||||||
const current = prev[id]?.status;
|
|
||||||
const next: Status | undefined =
|
|
||||||
current === undefined ? "doing" : current === "doing" ? "done" : undefined;
|
|
||||||
const merged: Progress = {
|
|
||||||
...prev[id],
|
|
||||||
status: next,
|
|
||||||
at: new Date().toISOString(),
|
|
||||||
};
|
|
||||||
if (isEmpty(merged)) {
|
|
||||||
const { [id]: _removed, ...rest } = prev;
|
|
||||||
return rest;
|
|
||||||
}
|
|
||||||
return { ...prev, [id]: merged };
|
|
||||||
});
|
|
||||||
},
|
|
||||||
[],
|
|
||||||
);
|
|
||||||
|
|
||||||
const setDaily = useCallback(
|
const setDaily = useCallback(
|
||||||
(id: string, daily: boolean | undefined) => patch(id, { daily }),
|
(id: string, daily: boolean | undefined) => patch(id, { daily }),
|
||||||
@@ -146,7 +129,6 @@ export function useProgress() {
|
|||||||
progress,
|
progress,
|
||||||
patch,
|
patch,
|
||||||
setStatus,
|
setStatus,
|
||||||
cycleStatus,
|
|
||||||
setDaily,
|
setDaily,
|
||||||
setEffort,
|
setEffort,
|
||||||
setNote,
|
setNote,
|
||||||
|
|||||||
Reference in New Issue
Block a user