fix(profile): don't clobber edits with a slow fetch; show decoration load errors
Pronouns, timezone and avatar decoration applied the mount-time fetch result unconditionally, overwriting a value the user had already edited; the decoration panel also showed "None" on any fetch failure and let the user save over a real decoration. Add cancelled/dirty guards (mirroring ProfileStatus) and an explicit load-error state with Retry. Fixes #46 Fixes #47 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -751,22 +751,32 @@ function ProfilePronouns() {
|
|||||||
|
|
||||||
const [pronouns, setPronouns] = useState<string>('');
|
const [pronouns, setPronouns] = useState<string>('');
|
||||||
const [savedPronouns, setSavedPronouns] = useState<string>('');
|
const [savedPronouns, setSavedPronouns] = useState<string>('');
|
||||||
|
// True once the user has edited the field — guards against the mount-time
|
||||||
|
// fetch below clobbering a fresh edit if it resolves late (mirrors
|
||||||
|
// ProfileStatus's statusDirtyRef in this file).
|
||||||
|
const pronounsDirtyRef = useRef(false);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
let cancelled = false;
|
||||||
mx.http
|
mx.http
|
||||||
.authedRequest<{ 'm.pronouns': string }>(
|
.authedRequest<{ 'm.pronouns': string }>(
|
||||||
Method.Get,
|
Method.Get,
|
||||||
`/profile/${encodeURIComponent(userId)}/m.pronouns`,
|
`/profile/${encodeURIComponent(userId)}/m.pronouns`,
|
||||||
)
|
)
|
||||||
.then((res) => {
|
.then((res) => {
|
||||||
|
if (cancelled || pronounsDirtyRef.current) return;
|
||||||
const val = res['m.pronouns'] ?? '';
|
const val = res['m.pronouns'] ?? '';
|
||||||
setPronouns(val);
|
setPronouns(val);
|
||||||
setSavedPronouns(val);
|
setSavedPronouns(val);
|
||||||
})
|
})
|
||||||
.catch(() => {
|
.catch(() => {
|
||||||
|
if (cancelled || pronounsDirtyRef.current) return;
|
||||||
setPronouns('');
|
setPronouns('');
|
||||||
setSavedPronouns('');
|
setSavedPronouns('');
|
||||||
});
|
});
|
||||||
|
return () => {
|
||||||
|
cancelled = true;
|
||||||
|
};
|
||||||
}, [mx, userId]);
|
}, [mx, userId]);
|
||||||
|
|
||||||
const [saveState, savePronouns] = useAsyncCallback(
|
const [saveState, savePronouns] = useAsyncCallback(
|
||||||
@@ -788,10 +798,12 @@ function ProfilePronouns() {
|
|||||||
const saving = saveState.status === AsyncStatus.Loading;
|
const saving = saveState.status === AsyncStatus.Loading;
|
||||||
|
|
||||||
const handleChange: ChangeEventHandler<HTMLInputElement> = (evt) => {
|
const handleChange: ChangeEventHandler<HTMLInputElement> = (evt) => {
|
||||||
|
pronounsDirtyRef.current = true;
|
||||||
setPronouns(evt.currentTarget.value);
|
setPronouns(evt.currentTarget.value);
|
||||||
};
|
};
|
||||||
|
|
||||||
const handleReset = () => {
|
const handleReset = () => {
|
||||||
|
pronounsDirtyRef.current = true;
|
||||||
setPronouns(savedPronouns);
|
setPronouns(savedPronouns);
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -875,10 +887,15 @@ function ProfileTimezone() {
|
|||||||
|
|
||||||
const [timezone, setTimezone] = useState<string>('');
|
const [timezone, setTimezone] = useState<string>('');
|
||||||
const [savedTimezone, setSavedTimezone] = useState<string>('');
|
const [savedTimezone, setSavedTimezone] = useState<string>('');
|
||||||
|
// True once the user has edited the field — guards against the mount-time
|
||||||
|
// fetch below clobbering a fresh edit if it resolves late (mirrors
|
||||||
|
// ProfileStatus's statusDirtyRef in this file).
|
||||||
|
const timezoneDirtyRef = useRef(false);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
let cancelled = false;
|
||||||
const cached = getAccountData<{ timezone: string }>(mx, 'im.lotus.timezone');
|
const cached = getAccountData<{ timezone: string }>(mx, 'im.lotus.timezone');
|
||||||
if (cached?.timezone) {
|
if (cached?.timezone && !timezoneDirtyRef.current) {
|
||||||
setTimezone(cached.timezone);
|
setTimezone(cached.timezone);
|
||||||
setSavedTimezone(cached.timezone);
|
setSavedTimezone(cached.timezone);
|
||||||
}
|
}
|
||||||
@@ -889,6 +906,7 @@ function ProfileTimezone() {
|
|||||||
`/user/${encodeURIComponent(userId)}/account_data/im.lotus.timezone`,
|
`/user/${encodeURIComponent(userId)}/account_data/im.lotus.timezone`,
|
||||||
)
|
)
|
||||||
.then((res) => {
|
.then((res) => {
|
||||||
|
if (cancelled || timezoneDirtyRef.current) return;
|
||||||
const val = res.timezone ?? '';
|
const val = res.timezone ?? '';
|
||||||
setTimezone(val);
|
setTimezone(val);
|
||||||
setSavedTimezone(val);
|
setSavedTimezone(val);
|
||||||
@@ -896,6 +914,9 @@ function ProfileTimezone() {
|
|||||||
.catch(() => {
|
.catch(() => {
|
||||||
/* no stored timezone yet */
|
/* no stored timezone yet */
|
||||||
});
|
});
|
||||||
|
return () => {
|
||||||
|
cancelled = true;
|
||||||
|
};
|
||||||
}, [mx, userId]);
|
}, [mx, userId]);
|
||||||
|
|
||||||
const [saveState, saveTimezone] = useAsyncCallback(
|
const [saveState, saveTimezone] = useAsyncCallback(
|
||||||
@@ -921,7 +942,13 @@ function ProfileTimezone() {
|
|||||||
);
|
);
|
||||||
const saving = saveState.status === AsyncStatus.Loading;
|
const saving = saveState.status === AsyncStatus.Loading;
|
||||||
|
|
||||||
|
const handleChange = (value: string) => {
|
||||||
|
timezoneDirtyRef.current = true;
|
||||||
|
setTimezone(value);
|
||||||
|
};
|
||||||
|
|
||||||
const handleReset = () => {
|
const handleReset = () => {
|
||||||
|
timezoneDirtyRef.current = true;
|
||||||
setTimezone(savedTimezone);
|
setTimezone(savedTimezone);
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -955,7 +982,7 @@ function ProfileTimezone() {
|
|||||||
{ value: '', label: '— select timezone —' },
|
{ value: '', label: '— select timezone —' },
|
||||||
...COMMON_TIMEZONES.map((tz) => ({ value: tz, label: tz })),
|
...COMMON_TIMEZONES.map((tz) => ({ value: tz, label: tz })),
|
||||||
]}
|
]}
|
||||||
onChange={setTimezone}
|
onChange={handleChange}
|
||||||
disabled={saving}
|
disabled={saving}
|
||||||
aria-label="Timezone"
|
aria-label="Timezone"
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import React, { useCallback, useEffect, useState } from 'react';
|
import React, { useCallback, useEffect, useRef, useState } from 'react';
|
||||||
import { Box, Button, Text, Spinner, color } from 'folds';
|
import { Box, Button, Text, Spinner, color } from 'folds';
|
||||||
import { Method } from 'matrix-js-sdk';
|
import { Method } from 'matrix-js-sdk';
|
||||||
import { useMatrixClient } from '../../../hooks/useMatrixClient';
|
import { useMatrixClient } from '../../../hooks/useMatrixClient';
|
||||||
@@ -67,24 +67,47 @@ export function ProfileDecoration() {
|
|||||||
|
|
||||||
const [current, setCurrent] = useState<string | null>(null);
|
const [current, setCurrent] = useState<string | null>(null);
|
||||||
const [selected, setSelected] = useState<string | null>(null);
|
const [selected, setSelected] = useState<string | null>(null);
|
||||||
|
// Distinguish "confirmed no decoration" from "failed to load": a fetch
|
||||||
|
// failure must not be shown as (and saved over) "None".
|
||||||
|
const [loadError, setLoadError] = useState(false);
|
||||||
|
const [loading, setLoading] = useState(true);
|
||||||
|
// True once the user has picked/cleared a decoration — guards against the
|
||||||
|
// mount-time fetch below clobbering a fresh selection if it resolves late
|
||||||
|
// (mirrors ProfileStatus's statusDirtyRef in Profile.tsx).
|
||||||
|
const dirtyRef = useRef(false);
|
||||||
|
|
||||||
useEffect(() => {
|
const fetchDecoration = useCallback(() => {
|
||||||
|
let cancelled = false;
|
||||||
|
setLoading(true);
|
||||||
// Fetch the whole profile, not the `/{field}` sub-resource: an unset field
|
// Fetch the whole profile, not the `/{field}` sub-resource: an unset field
|
||||||
// 404s (a console error for anyone without a decoration). The full profile
|
// 404s (a console error for anyone without a decoration). The full profile
|
||||||
// returns 200 with all fields incl. custom MSC4133 ones — read it out.
|
// returns 200 with all fields incl. custom MSC4133 ones — read it out.
|
||||||
mx.http
|
mx.http
|
||||||
.authedRequest<Record<string, string>>(Method.Get, `/profile/${encodeURIComponent(userId)}`)
|
.authedRequest<Record<string, string>>(Method.Get, `/profile/${encodeURIComponent(userId)}`)
|
||||||
.then((res) => {
|
.then((res) => {
|
||||||
|
if (cancelled) return;
|
||||||
|
setLoadError(false);
|
||||||
|
setLoading(false);
|
||||||
|
if (dirtyRef.current) return;
|
||||||
const val = (res[PROFILE_FIELD] as string | undefined) ?? null;
|
const val = (res[PROFILE_FIELD] as string | undefined) ?? null;
|
||||||
setCurrent(val);
|
setCurrent(val);
|
||||||
setSelected(val);
|
setSelected(val);
|
||||||
})
|
})
|
||||||
.catch(() => {
|
.catch(() => {
|
||||||
setCurrent(null);
|
if (cancelled) return;
|
||||||
setSelected(null);
|
setLoading(false);
|
||||||
|
// Do NOT touch current/selected here — a network failure is not proof
|
||||||
|
// there's no decoration, and defaulting to null risks the user saving
|
||||||
|
// "None" over a real, still-set decoration (see #46).
|
||||||
|
setLoadError(true);
|
||||||
});
|
});
|
||||||
|
return () => {
|
||||||
|
cancelled = true;
|
||||||
|
};
|
||||||
}, [mx, userId]);
|
}, [mx, userId]);
|
||||||
|
|
||||||
|
useEffect(() => fetchDecoration(), [fetchDecoration]);
|
||||||
|
|
||||||
const [saveState, save] = useAsyncCallback(
|
const [saveState, save] = useAsyncCallback(
|
||||||
useCallback(
|
useCallback(
|
||||||
async (slug: string | null) => {
|
async (slug: string | null) => {
|
||||||
@@ -105,16 +128,27 @@ export function ProfileDecoration() {
|
|||||||
const hasChanges = selected !== current;
|
const hasChanges = selected !== current;
|
||||||
|
|
||||||
const handleSelect = (slug: string) => {
|
const handleSelect = (slug: string) => {
|
||||||
|
dirtyRef.current = true;
|
||||||
setSelected((prev) => (prev === slug ? null : slug));
|
setSelected((prev) => (prev === slug ? null : slug));
|
||||||
};
|
};
|
||||||
|
|
||||||
const handleClear = () => setSelected(null);
|
const handleClear = () => {
|
||||||
|
dirtyRef.current = true;
|
||||||
|
setSelected(null);
|
||||||
|
};
|
||||||
|
|
||||||
const handleSave = () => {
|
const handleSave = () => {
|
||||||
if (!hasChanges || saving) return;
|
// Refuse to save while the initial load failed: `current`/`selected` are
|
||||||
|
// not known-good, so saving could silently overwrite a real decoration.
|
||||||
|
if (!hasChanges || saving || loadError) return;
|
||||||
save(selected);
|
save(selected);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const handleRetry = () => {
|
||||||
|
dirtyRef.current = false;
|
||||||
|
fetchDecoration();
|
||||||
|
};
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<SettingTile
|
<SettingTile
|
||||||
title={
|
title={
|
||||||
@@ -159,13 +193,15 @@ export function ProfileDecoration() {
|
|||||||
</div>
|
</div>
|
||||||
<Box grow="Yes" direction="Column" gap="100">
|
<Box grow="Yes" direction="Column" gap="100">
|
||||||
<Text size="T300">
|
<Text size="T300">
|
||||||
{selected
|
{loadError
|
||||||
|
? 'Failed to load'
|
||||||
|
: selected
|
||||||
? (DECORATION_CATEGORIES.flatMap((c) => c.decorations).find(
|
? (DECORATION_CATEGORIES.flatMap((c) => c.decorations).find(
|
||||||
(d) => d.slug === selected,
|
(d) => d.slug === selected,
|
||||||
)?.name ?? selected)
|
)?.name ?? selected)
|
||||||
: 'None'}
|
: 'None'}
|
||||||
</Text>
|
</Text>
|
||||||
{selected && (
|
{selected && !loadError && (
|
||||||
<Button
|
<Button
|
||||||
type="button"
|
type="button"
|
||||||
size="300"
|
size="300"
|
||||||
@@ -178,7 +214,7 @@ export function ProfileDecoration() {
|
|||||||
</Button>
|
</Button>
|
||||||
)}
|
)}
|
||||||
</Box>
|
</Box>
|
||||||
{hasChanges && (
|
{hasChanges && !loadError && (
|
||||||
<Button
|
<Button
|
||||||
type="button"
|
type="button"
|
||||||
size="400"
|
size="400"
|
||||||
@@ -194,6 +230,26 @@ export function ProfileDecoration() {
|
|||||||
)}
|
)}
|
||||||
</Box>
|
</Box>
|
||||||
|
|
||||||
|
{loadError && (
|
||||||
|
<Box alignItems="Center" gap="200">
|
||||||
|
<Text size="T200" style={{ color: color.Critical.Main }}>
|
||||||
|
Could not load your current decoration. Saving is disabled until this succeeds, so you
|
||||||
|
don’t overwrite it based on a wrong display.
|
||||||
|
</Text>
|
||||||
|
<Button
|
||||||
|
type="button"
|
||||||
|
size="300"
|
||||||
|
radii="300"
|
||||||
|
variant="Secondary"
|
||||||
|
fill="Soft"
|
||||||
|
onClick={handleRetry}
|
||||||
|
disabled={loading}
|
||||||
|
>
|
||||||
|
<Text size="B300">{loading ? 'Retrying…' : 'Retry'}</Text>
|
||||||
|
</Button>
|
||||||
|
</Box>
|
||||||
|
)}
|
||||||
|
|
||||||
{saveState.status === AsyncStatus.Error && (
|
{saveState.status === AsyncStatus.Error && (
|
||||||
<Text size="T200" style={{ color: color.Critical.Main }}>
|
<Text size="T200" style={{ color: color.Critical.Main }}>
|
||||||
Failed to save. Try again.
|
Failed to save. Try again.
|
||||||
|
|||||||
Reference in New Issue
Block a user