Migrate /items/[itemId] to Next routing

The tricky part here was that `returnPartialData` seems to behave differently during SSR. On the page itself, this seems to cause us to always get back at least an empty object, but in SSR we can sometimes get null—which means that a LOT of code that expects the item object to exist while in loading state gets thrown off.

To keep this situation maximally clear, I added a bunch of null handling with `?.` to `ItemPageLayout`. An alternative would have been to check for null and put in an empty object if not, but this feels more resilient and more true to the situation.

The search bar here is a bit tricky, but is pretty straightforwardly adapted from how we did the layouts in App.js. Fingers crossed that it works as smoothly as expected when the search page is migrated too! (Right now typing in there is all messy because it hops over to the fallback route and does its whole separate thing.)
This commit is contained in:
Emi Matchu 2022-09-14 22:16:39 -07:00
parent 1dbc142f4f
commit 43ae248e87
7 changed files with 113 additions and 78 deletions

19
pages/items/[itemId].tsx Normal file
View file

@ -0,0 +1,19 @@
import ItemSearchPageToolbar from "../../src/app/components/ItemSearchPageToolbar";
import ItemPage from "../../src/app/ItemPage";
import PageLayout from "../../src/app/PageLayout";
import type { NextPageWithLayout } from "../_app";
const ItemPageWrapper: NextPageWithLayout = () => {
return <ItemPage />;
};
ItemPageWrapper.layoutComponent = ({ children }) => {
return (
<PageLayout>
<ItemSearchPageToolbar marginBottom="8" />
{children}
</PageLayout>
);
};
export default ItemPageWrapper;

View file

@ -13,7 +13,6 @@ import { loadable } from "./util";
const HomePage = loadable(() => import("./HomePage")); const HomePage = loadable(() => import("./HomePage"));
const ItemSearchPage = loadable(() => import("./ItemSearchPage")); const ItemSearchPage = loadable(() => import("./ItemSearchPage"));
const ItemPage = loadable(() => import("./ItemPage"));
const ItemTradesOfferingPage = loadable(() => const ItemTradesOfferingPage = loadable(() =>
import("./ItemTradesPage").then((m) => m.ItemTradesOfferingPage) import("./ItemTradesPage").then((m) => m.ItemTradesOfferingPage)
); );
@ -67,12 +66,6 @@ function App() {
<ItemTradesSeekingPage /> <ItemTradesSeekingPage />
</PageLayout> </PageLayout>
</Route> </Route>
<Route path="/items/:itemId">
<PageLayout>
<ItemSearchPageToolbar marginBottom="8" />
<ItemPage />
</PageLayout>
</Route>
<Route path="/outfits/new"> <Route path="/outfits/new">
<WardrobePage /> <WardrobePage />
</Route> </Route>

View file

@ -114,11 +114,11 @@ function ColorModeButton() {
function useClassicDTIUrl() { function useClassicDTIUrl() {
const { pathname, query } = useRouter(); const { pathname, query } = useRouter();
if (pathname === "/items/:itemId") { if (pathname === "/items/[itemId]") {
return `https://impress.openneo.net/items/${query.itemId}`; return `https://impress.openneo.net/items/${query.itemId}`;
} }
if (pathname === "/user/:userId/lists") { if (pathname === "/user/[userId]/lists") {
return `https://impress.openneo.net/user/${query.userId}/closet`; return `https://impress.openneo.net/user/${query.userId}/closet`;
} }

View file

@ -33,7 +33,7 @@ import {
import { MdPause, MdPlayArrow } from "react-icons/md"; import { MdPause, MdPlayArrow } from "react-icons/md";
import gql from "graphql-tag"; import gql from "graphql-tag";
import { useQuery, useMutation } from "@apollo/client"; import { useQuery, useMutation } from "@apollo/client";
import { Link, useParams } from "react-router-dom"; import Link from "next/link";
import ItemPageLayout, { SubtleSkeleton } from "./ItemPageLayout"; import ItemPageLayout, { SubtleSkeleton } from "./ItemPageLayout";
import { import {
@ -58,10 +58,11 @@ import useCurrentUser from "./components/useCurrentUser";
import SpeciesFacesPicker, { import SpeciesFacesPicker, {
colorIsBasic, colorIsBasic,
} from "./ItemPage/SpeciesFacesPicker"; } from "./ItemPage/SpeciesFacesPicker";
import { useRouter } from "next/router";
function ItemPage() { function ItemPage() {
const { itemId } = useParams(); const { query } = useRouter();
return <ItemPageContent itemId={itemId} />; return <ItemPageContent itemId={query.itemId} />;
} }
/** /**
@ -69,7 +70,7 @@ function ItemPage() {
* entry point for ItemPageDrawer! When embedded in ItemPageDrawer, the * entry point for ItemPageDrawer! When embedded in ItemPageDrawer, the
* `isEmbedded` prop is true, so we know not to e.g. set the page title. * `isEmbedded` prop is true, so we know not to e.g. set the page title.
*/ */
export function ItemPageContent({ itemId, isEmbedded }) { export function ItemPageContent({ itemId, isEmbedded = false }) {
const { isLoggedIn } = useCurrentUser(); const { isLoggedIn } = useCurrentUser();
const { error, data } = useQuery( const { error, data } = useQuery(
@ -706,30 +707,31 @@ function ItemPageTradeLinks({ itemId, isEmbedded }) {
function ItemPageTradeLink({ href, count, label, colorScheme, isEmbedded }) { function ItemPageTradeLink({ href, count, label, colorScheme, isEmbedded }) {
return ( return (
<Button <Link href={href} passHref>
as={Link} <Button
to={href} as="a"
target={isEmbedded ? "_blank" : undefined} target={isEmbedded ? "_blank" : undefined}
size="xs" size="xs"
variant="outline" variant="outline"
colorScheme={colorScheme} colorScheme={colorScheme}
borderRadius="full" borderRadius="full"
paddingRight="1" paddingRight="1"
> >
<Box display="grid" gridTemplateAreas="single-area"> <Box display="grid" gridTemplateAreas="single-area">
<Box gridArea="single-area" display="flex" justifyContent="center"> <Box gridArea="single-area" display="flex" justifyContent="center">
{count} {label} <ChevronRightIcon minHeight="1.2em" /> {count} {label} <ChevronRightIcon minHeight="1.2em" />
</Box>
<Box
gridArea="single-area"
display="flex"
justifyContent="center"
visibility="hidden"
>
888 offering <ChevronRightIcon minHeight="1.2em" />
</Box>
</Box> </Box>
<Box </Button>
gridArea="single-area" </Link>
display="flex"
justifyContent="center"
visibility="hidden"
>
888 offering <ChevronRightIcon minHeight="1.2em" />
</Box>
</Box>
</Button>
); );
} }
@ -1149,9 +1151,8 @@ function CustomizeMoreButton({ speciesId, colorId, pose, itemId, isDisabled }) {
const backgroundColorHover = useColorModeValue(undefined, "blackAlpha.900"); const backgroundColorHover = useColorModeValue(undefined, "blackAlpha.900");
return ( return (
<Button <LinkOrButton
as={isDisabled ? "button" : Link} href={isDisabled ? null : url}
to={isDisabled ? null : url}
role="group" role="group"
position="absolute" position="absolute"
top="2" top="2"
@ -1165,10 +1166,22 @@ function CustomizeMoreButton({ speciesId, colorId, pose, itemId, isDisabled }) {
> >
<ExpandOnGroupHover paddingRight="2">Customize more</ExpandOnGroupHover> <ExpandOnGroupHover paddingRight="2">Customize more</ExpandOnGroupHover>
<EditIcon /> <EditIcon />
</Button> </LinkOrButton>
); );
} }
function LinkOrButton({ href, ...props }) {
if (href != null) {
return (
<Link href={href} passHref>
<Button as="a" {...props} />
</Link>
);
} else {
return <Button {...props} />;
}
}
/** /**
* ExpandOnGroupHover starts at width=0, and expands to full width when a * ExpandOnGroupHover starts at width=0, and expands to full width when a
* parent with role="group" gains hover or focus state. * parent with role="group" gains hover or focus state.

View file

@ -113,11 +113,11 @@ function ItemPageBadges({ item, isEmbedded }) {
</SubtleSkeleton> </SubtleSkeleton>
{ {
// If the createdAt date is null (loaded and empty), hide the badge. // If the createdAt date is null (loaded and empty), hide the badge.
item.createdAt !== null && ( item?.createdAt !== null && (
<SubtleSkeleton <SubtleSkeleton
// Distinguish between undefined (still loading) and null (loaded and // Distinguish between undefined (still loading) and null (loaded and
// empty). // empty).
isLoaded={item.createdAt !== undefined} isLoaded={item?.createdAt !== undefined}
> >
<Badge <Badge
display="block" display="block"
@ -125,14 +125,14 @@ function ItemPageBadges({ item, isEmbedded }) {
boxSizing="content-box" boxSizing="content-box"
textAlign="center" textAlign="center"
> >
{item.createdAt && <ShortTimestamp when={item.createdAt} />} {item?.createdAt && <ShortTimestamp when={item?.createdAt} />}
</Badge> </Badge>
</SubtleSkeleton> </SubtleSkeleton>
) )
} }
<SubtleSkeleton isLoaded={searchBadgesAreLoaded}> <SubtleSkeleton isLoaded={searchBadgesAreLoaded}>
<LinkBadge <LinkBadge
href={`https://impress.openneo.net/items/${item.id}`} href={`https://impress.openneo.net/items/${item?.id}`}
isEmbedded={isEmbedded} isEmbedded={isEmbedded}
> >
Classic DTI Classic DTI
@ -142,7 +142,7 @@ function ItemPageBadges({ item, isEmbedded }) {
<LinkBadge <LinkBadge
href={ href={
"https://items.jellyneo.net/search/?name=" + "https://items.jellyneo.net/search/?name=" +
encodeURIComponent(item.name) + encodeURIComponent(item?.name) +
"&name_type=3" "&name_type=3"
} }
isEmbedded={isEmbedded} isEmbedded={isEmbedded}
@ -150,17 +150,17 @@ function ItemPageBadges({ item, isEmbedded }) {
Jellyneo Jellyneo
</LinkBadge> </LinkBadge>
</SubtleSkeleton> </SubtleSkeleton>
{item.isNc && ( {item?.isNc && (
<SubtleSkeleton <SubtleSkeleton
isLoaded={ isLoaded={
// Distinguish between undefined (still loading) and null (loaded // Distinguish between undefined (still loading) and null (loaded
// and empty). // and empty).
item.ncTradeValueText !== undefined item?.ncTradeValueText !== undefined
} }
> >
{item.ncTradeValueText && ( {item?.ncTradeValueText && (
<LinkBadge href="http://www.neopets.com/~owls"> <LinkBadge href="http://www.neopets.com/~owls">
OWLS: {item.ncTradeValueText} OWLS: {item?.ncTradeValueText}
</LinkBadge> </LinkBadge>
)} )}
</SubtleSkeleton> </SubtleSkeleton>
@ -170,7 +170,7 @@ function ItemPageBadges({ item, isEmbedded }) {
<LinkBadge <LinkBadge
href={ href={
"http://www.neopets.com/shops/wizard.phtml?string=" + "http://www.neopets.com/shops/wizard.phtml?string=" +
encodeURIComponent(item.name) encodeURIComponent(item?.name)
} }
isEmbedded={isEmbedded} isEmbedded={isEmbedded}
> >
@ -183,7 +183,7 @@ function ItemPageBadges({ item, isEmbedded }) {
<LinkBadge <LinkBadge
href={ href={
"http://www.neopets.com/portal/supershopwiz.phtml?string=" + "http://www.neopets.com/portal/supershopwiz.phtml?string=" +
encodeURIComponent(item.name) encodeURIComponent(item?.name)
} }
isEmbedded={isEmbedded} isEmbedded={isEmbedded}
> >
@ -196,7 +196,7 @@ function ItemPageBadges({ item, isEmbedded }) {
<LinkBadge <LinkBadge
href={ href={
"http://www.neopets.com/island/tradingpost.phtml?type=browse&criteria=item_exact&search_string=" + "http://www.neopets.com/island/tradingpost.phtml?type=browse&criteria=item_exact&search_string=" +
encodeURIComponent(item.name) encodeURIComponent(item?.name)
} }
isEmbedded={isEmbedded} isEmbedded={isEmbedded}
> >
@ -209,7 +209,7 @@ function ItemPageBadges({ item, isEmbedded }) {
<LinkBadge <LinkBadge
href={ href={
"http://www.neopets.com/genie.phtml?type=process_genie&criteria=exact&auctiongenie=" + "http://www.neopets.com/genie.phtml?type=process_genie&criteria=exact&auctiongenie=" +
encodeURIComponent(item.name) encodeURIComponent(item?.name)
} }
isEmbedded={isEmbedded} isEmbedded={isEmbedded}
> >
@ -228,7 +228,7 @@ function ItemKindBadgeWithSupportTools({ item }) {
const ncRef = React.useRef(null); const ncRef = React.useRef(null);
const isNcAutoDetectedFromRarity = const isNcAutoDetectedFromRarity =
item.rarityIndex === 500 || item.rarityIndex === 0; item?.rarityIndex === 500 || item?.rarityIndex === 0;
const [mutate, { loading }] = useMutation(gql` const [mutate, { loading }] = useMutation(gql`
mutation ItemPageSupportSetIsManuallyNc( mutation ItemPageSupportSetIsManuallyNc(
@ -248,7 +248,11 @@ function ItemKindBadgeWithSupportTools({ item }) {
} }
`); `);
if (isSupportUser && item.rarityIndex != null && item.isManuallyNc != null) { if (
isSupportUser &&
item?.rarityIndex != null &&
item?.isManuallyNc != null
) {
// TODO: Could code-split this into a SupportOnly file... // TODO: Could code-split this into a SupportOnly file...
return ( return (
<Popover placement="bottom-start" initialFocusRef={ncRef} showArrow> <Popover placement="bottom-start" initialFocusRef={ncRef} showArrow>
@ -329,7 +333,7 @@ function ItemKindBadgeWithSupportTools({ item }) {
); );
} }
return <ItemKindBadge isNc={item.isNc} isPb={item.isPb} />; return <ItemKindBadge isNc={item?.isNc} isPb={item?.isPb} />;
} }
const LinkBadge = React.forwardRef( const LinkBadge = React.forwardRef(

View file

@ -164,13 +164,16 @@ export function ItemThumbnail({
}, },
])} ])}
> >
<Box {/* If the item is still loading, wait with an empty box. */}
as="img" {item && (
width="100%" <Box
height="100%" as="img"
src={safeImageUrl(item.thumbnailUrl)} width="100%"
alt={`Thumbnail art for ${item.name}`} height="100%"
/> src={safeImageUrl(item.thumbnailUrl)}
alt={`Thumbnail art for ${item.name}`}
/>
)}
</Box> </Box>
</Box> </Box>
)} )}

View file

@ -1,7 +1,7 @@
import React from "react"; import React from "react";
import { useHistory, useLocation, useParams } from "react-router-dom";
import { useCommonStyles } from "../util"; import { useCommonStyles } from "../util";
import SearchToolbar from "../WardrobePage/SearchToolbar"; import SearchToolbar from "../WardrobePage/SearchToolbar";
import { useRouter } from "next/router";
function ItemSearchPageToolbar({ ...props }) { function ItemSearchPageToolbar({ ...props }) {
const { query, setQuery } = useSearchQueryInUrl(); const { query, setQuery } = useSearchQueryInUrl();
@ -24,27 +24,30 @@ function ItemSearchPageToolbar({ ...props }) {
* query in the URL! It also parses out the offset for us. * query in the URL! It also parses out the offset for us.
*/ */
export function useSearchQueryInUrl() { export function useSearchQueryInUrl() {
const history = useHistory(); const {
pathname,
const { query: value } = useParams(); query: queryParams,
const { pathname, search } = useLocation(); push: pushHistory,
replace: replaceHistory,
} = useRouter();
const value = queryParams.query;
// Parse the query from the location. (We memoize this because we use it as a // Parse the query from the location. (We memoize this because we use it as a
// dependency in the query-saving hook below.) // dependency in the query-saving hook below.)
const parsedQuery = React.useMemo(() => { const parsedQuery = React.useMemo(() => {
const searchParams = new URLSearchParams(search);
return { return {
value: decodeURIComponent(value || ""), value: decodeURIComponent(value || ""),
filterToZoneLabel: searchParams.get("zone") || null, filterToZoneLabel: queryParams.zone || null,
filterToItemKind: searchParams.get("kind") || null, filterToItemKind: queryParams.kind || null,
filterToCurrentUserOwnsOrWants: searchParams.get("user") || null, filterToCurrentUserOwnsOrWants: queryParams.user || null,
}; };
}, [search, value]); }, [queryParams.zone, queryParams.kind, queryParams.user, value]);
const offset = parseInt(new URLSearchParams(search).get("offset")) || 0; const offset = parseInt(queryParams.offset) || 0;
// While on the search page, save the most recent parsed query in state. // While on the search page, save the most recent parsed query in state.
const isSearchPage = pathname.startsWith("/items/search"); const isSearchPage =
pathname === "/items/search" || pathname === "/items/search/[query]";
const [savedQuery, setSavedQuery] = React.useState(parsedQuery); const [savedQuery, setSavedQuery] = React.useState(parsedQuery);
React.useEffect(() => { React.useEffect(() => {
if (isSearchPage) { if (isSearchPage) {
@ -88,14 +91,14 @@ export function useSearchQueryInUrl() {
// navigation, like if they see the results and decide it's the wrong // navigation, like if they see the results and decide it's the wrong
// thing. // thing.
if (isSearchPage) { if (isSearchPage) {
history.replace(url); replaceHistory(url);
} else { } else {
// When you use the search toolbar from the item page, treat it as a // When you use the search toolbar from the item page, treat it as a
// full navigation! // full navigation!
history.push(url); pushHistory(url);
} }
}, },
[history, isSearchPage] [replaceHistory, pushHistory, isSearchPage]
); );
// NOTE: We don't provide a `setOffset`, because that's handled via // NOTE: We don't provide a `setOffset`, because that's handled via