Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions src/components/library/molecules/Modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,20 @@ export function Modal(props: ModalProps): JSX.Element {
// Fire the real close, then drop back to the open state. If onClose
// unmounts us (the usual case) this re-render is discarded; if onClose was
// a guarded no-op (e.g. a confirm dialog is open on top), we revert instead
// of getting stuck faded-out-but-mounted.
// of getting stuck faded-out-but-mounted. A close that navigates (the
// object overview leaves by URL) unmounts us only once the route settles,
// so it hands back its promise and we stay faded out until then: reverting
// at once drew the dialog again for a beat before it went.
let cancelled = false;
const finish = () => {
onClose();
const pending = onClose();
if (pending && typeof pending.then === 'function') {
const revert = () => {
if (!cancelled) setIsClosing(false);
};
pending.then(revert, revert);
return;
}
setIsClosing(false);
};

Expand All @@ -85,11 +96,16 @@ export function Modal(props: ModalProps): JSX.Element {

if (prefersReducedMotion) {
finish();
return;
return () => {
cancelled = true;
};
}

const timer = window.setTimeout(finish, CLOSE_ANIMATION_MS);
return () => window.clearTimeout(timer);
return () => {
cancelled = true;
window.clearTimeout(timer);
};
}, [isClosing, onClose]);

const handleBackdropPointerDown = (
Expand Down
4 changes: 3 additions & 1 deletion src/components/library/molecules/Modal/Modal.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ export interface ModalProps {
children: ReactNode;
className?: string;
wrapperClassName?: string;
onClose: () => void;
// A close that finishes later (a route change) returns its promise; the
// modal stays faded out until it settles instead of flashing back.
onClose: () => void | Promise<unknown>;
// Modal assigns its animated-close fn here so a modal's own content buttons
// (Cancel/Close/etc.) can trigger the same fade-out the backdrop and Esc use.
closeRef?: MutableRefObject<(() => void) | null>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import {
} from '@utils/library/schema/editLibrarySchema';
import axios from 'axios';
import classNames from 'classnames';
import { useRouter } from 'next/router';
import React, { JSX, useMemo, useRef, useState } from 'react';
import { Controller, useForm } from 'react-hook-form';

import type { IUpdateLibraryPayload } from '@local-types/library/library';
import type { IUpdateMeErrorBody } from '@local-types/library/user';

import { libraryPath } from '@lib/library/libraryPath';
import { richTextLength, toEditorHtml } from '@lib/library/richText';

import { createLibrary } from '@api/library/createLibrary';
Expand Down Expand Up @@ -68,6 +70,7 @@ function readUsernameError(
export function EditLibraryModal(props: EditLibraryModalProps): JSX.Element {
const { className, library, onClose, onSaved } = props;
const { accountData, setAccountData } = useAuth();
const router = useRouter();

const currentAvatarUrl = absoluteUrl(
library?.attributes.avatar?.data?.attributes.url,
Expand Down Expand Up @@ -189,6 +192,36 @@ export function EditLibraryModal(props: EditLibraryModalProps): JSX.Element {
setAvatarError(null);
};

// The library's address is the owner's username. After a rename the page
// moves to the new address, and the account takes the new name in the same
// beat the page takes it: an empty library has no row yet, so the address
// is the only thing proving it is mine, and a render where the name and the
// address disagree read as "No such library".
const followRenamedLibrary = async (
freshUser: NonNullable<typeof accountData>,
) => {
let swapped = false;
const swap = () => {
if (swapped) return;
swapped = true;
setAccountData(freshUser);
};
const onLibraryPage = router.pathname.startsWith('/library/[username]');
if (onLibraryPage && freshUser.username) {
router.events.on('beforeHistoryChange', swap);
try {
await router.replace(libraryPath(freshUser.username), undefined, {
scroll: false,
});
} catch (error) {
console.error('EditLibraryModal could not follow the rename:', error);
} finally {
router.events.off('beforeHistoryChange', swap);
}
}
swap();
Comment on lines +210 to +222

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If router.replace rejects (e.g. a concurrent navigation cancels this one — the realistic case, since a failed getServerSideProps typically renders an error page rather than rejecting), the catch only logs and the unconditional swap() on line 222 still fires, so accountData.username moves to the new name while the URL is left on the old slug. That's the exact "name and address disagree" state this PR is fixing, just reachable again on the failure path.

Consider only swapping in the catch when you can confirm the address actually moved (e.g. check router.asPath against libraryPath(freshUser.username)), or surface an error instead of silently applying the rename to accountData.

Fix this →

};

const onSubmit = async (data: EditLibraryFormData) => {
if (saveInFlightRef.current) return;
saveInFlightRef.current = true;
Expand Down Expand Up @@ -284,7 +317,11 @@ export function EditLibraryModal(props: EditLibraryModalProps): JSX.Element {
// reloaded by the caller via the resolved id — a direct GET by id, which
// (unlike the owner relation-filter) reliably resolves a just-created row.
const freshUser = await getUserInfo();
if (freshUser) setAccountData(freshUser);
if (freshUser && usernameChanged) {
await followRenamedLibrary(freshUser);
} else if (freshUser) {
setAccountData(freshUser);
}
if (libraryId != null) onSaved?.(libraryId);
savedPending.current = true;
close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,12 @@
justify-self: end;
max-width: 100%;

// On desktop the search summary hangs out of flow in the row gap, directly
// above this control. Keep 10px clear between that line and the meter.
@media (min-width: 769px) {
margin-top: 10px;
}

@media (max-width: 768px) {
justify-self: stretch;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export function ObjectOverviewModal(
return;
}
if (deleteLoading || deleting) return;
onClose();
return onClose();
}, [deleteLoading, deleting, onClose]);

const { closeRef, close } = useModalClose(guardedOnClose);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface ObjectOverviewModalProps {
* Pass the *library owner*, not the viewer.
*/
ownerUsername: string;
onClose: () => void;
onClose: () => void | Promise<unknown>;
/**
* Sibling objects on the same shelf — passed straight to the edit modal so
* the reorder grid in step 2 can render the shelf's real contents.
Expand Down
21 changes: 13 additions & 8 deletions src/components/library/organisms/Shelf/Shelf.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -540,17 +540,22 @@ export function Shelf(props: ShelfProps): JSX.Element {
{ shallow: true, scroll: false },
);
};
// Returns the navigation so the overview stays faded out until the route
// settles and unmounts it.
const closeObject = () => {
if (onShareRoute) {
const rest = { ...router.query };
delete rest.o;
void router.push({ pathname: router.pathname, query: rest }, undefined, {
shallow: true,
scroll: false,
});
return;
return router.push(
{ pathname: router.pathname, query: rest },
undefined,
{
shallow: true,
scroll: false,
},
);
}
void router.push(libraryPath(ownerUsername || urlUsername), undefined, {
return router.push(libraryPath(ownerUsername || urlUsername), undefined, {
shallow: true,
scroll: false,
});
Expand Down Expand Up @@ -944,7 +949,7 @@ export function Shelf(props: ShelfProps): JSX.Element {
// close the overview so the user sees the move take effect.
if (newShelfId != null && newShelfId !== from) {
onObjectMoved?.(from, newShelfId, updated);
closeObject();
void closeObject();
return;
}
// No need to track the object locally — it flows back through `objects` and
Expand All @@ -954,7 +959,7 @@ export function Shelf(props: ShelfProps): JSX.Element {

const handleDeleted = (id: number) => {
onObjectDeleted?.(homeShelfId(id), id);
closeObject();
void closeObject();
};

return (
Expand Down
Loading