Skip to content

Commit d6d1889

Browse files
committed
fix(webapp): address review feedback on favorites and sidebar customization
Skip the locked preference transaction on the hot current-environment save when the session snapshot already matches, so plain page loads do no extra database work. Give favorite rename and remove their own fetchers so quick successive mutations can't cancel each other. Close favorite and More popovers when navigation changes only the search string, keep the section options trigger visible while it holds keyboard focus, and dedupe saved order ids defensively.
1 parent 78d301b commit d6d1889

5 files changed

Lines changed: 24 additions & 7 deletions

File tree

apps/webapp/app/components/navigation/FavoritesSection.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ export function FavoriteMenuItem({
4545
const [isEditing, setIsEditing] = useState(false);
4646
const [isMenuOpen, setMenuOpen] = useState(false);
4747

48+
// Watch search too: navigating to a favorite can change only the search on the same pathname
4849
useEffect(() => {
4950
setMenuOpen(false);
50-
}, [navigation.location?.pathname]);
51+
}, [navigation.location?.pathname, navigation.location?.search]);
5152

5253
const icon = favoritePageIcon(favorite.icon);
5354
const isActive = isFavoriteActive(favorite, location.pathname, location.search);

apps/webapp/app/components/navigation/SideMenu.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,15 +406,18 @@ export function SideMenu({
406406
};
407407
// Same ownership rule: removing a favorite optimistically unmounts its menu item (and, for the
408408
// last favorite, the whole section), which would abort an item-owned fetcher mid-request.
409-
const favoriteActionsFetcher = useFetcher();
409+
// Separate fetchers per mutation: fetchers are single-flight, so a shared one would cancel an
410+
// in-flight rename when a remove follows quickly (or vice versa).
411+
const removeFavoriteFetcher = useFetcher();
412+
const renameFavoriteFetcher = useFetcher();
410413
const removeFavorite = (id: string) => {
411-
favoriteActionsFetcher.submit(
414+
removeFavoriteFetcher.submit(
412415
{ intent: "remove", id },
413416
{ method: "POST", action: FAVORITES_ACTION_PATH }
414417
);
415418
};
416419
const renameFavorite = (id: string, label: string) => {
417-
favoriteActionsFetcher.submit(
420+
renameFavoriteFetcher.submit(
418421
{ intent: "rename", id, label },
419422
{ method: "POST", action: FAVORITES_ACTION_PATH }
420423
);
@@ -1379,9 +1382,10 @@ function SideMenuMoreItem({
13791382
const [isOpen, setOpen] = useState(false);
13801383
const navigation = useNavigation();
13811384

1385+
// Watch search too: navigating to a favorite can change only the search on the same pathname
13821386
useEffect(() => {
13831387
setOpen(false);
1384-
}, [navigation.location?.pathname]);
1388+
}, [navigation.location?.pathname, navigation.location?.search]);
13851389

13861390
return (
13871391
<Popover open={isOpen} onOpenChange={setOpen}>

apps/webapp/app/components/navigation/SideMenuSection.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ export function SideMenuSection({
9090
className="absolute right-1 top-1/2 -translate-y-1/2"
9191
style={{ opacity: "var(--sm-label-opacity, 1)" }}
9292
>
93-
<div className="opacity-0 has-[[data-state=open]]:opacity-100 group-hover/sectionheader:opacity-100">
93+
{/* focus-within keeps the trigger visible for keyboard users tabbing onto it */}
94+
<div className="opacity-0 focus-within:opacity-100 has-[[data-state=open]]:opacity-100 group-hover/sectionheader:opacity-100">
9495
{headerMenu}
9596
</div>
9697
</div>

apps/webapp/app/components/navigation/sideMenuTypes.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ export function orderByPreference<T extends { id: string }>(
4141
if (!savedOrder || savedOrder.length === 0) return entries;
4242

4343
const defaultIndex = new Map(entries.map((entry, index) => [entry.id, index]));
44-
const orderedIds = savedOrder.filter((id) => defaultIndex.has(id));
44+
// Set-dedupe: a corrupted saved order with duplicate ids must not render an entry twice
45+
const orderedIds = [...new Set(savedOrder.filter((id) => defaultIndex.has(id)))];
4546
const missingIds = entries.map((entry) => entry.id).filter((id) => !orderedIds.includes(id));
4647

4748
for (const id of missingIds) {

apps/webapp/app/services/dashboardPreferences.server.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,16 @@ export async function updateCurrentProjectEnvironmentId({
130130
return;
131131
}
132132

133+
// Fast path: this runs on nearly every navigation (env layout loader), so skip the locked
134+
// transaction when the session snapshot already matches. The in-transaction check below stays
135+
// authoritative for the rare stale-snapshot case.
136+
if (
137+
user.dashboardPreferences.currentProjectId === projectId &&
138+
user.dashboardPreferences.projects[projectId]?.currentEnvironment?.id === environmentId
139+
) {
140+
return;
141+
}
142+
133143
return mutateDashboardPreferences(user.id, (prefs) => {
134144
//only update if the existing preferences are different
135145
if (

0 commit comments

Comments
 (0)