-
Notifications
You must be signed in to change notification settings - Fork 0
Aside state persisted in query params #189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,20 @@ | ||
| "use client"; | ||
|
|
||
| import { ClassListView } from "@/components/classes/list/class-list-view"; | ||
| import { PageLayout } from "@/components/page-layout"; | ||
| import { parseAsString, useQueryState } from "nuqs"; | ||
|
|
||
| export default function ClassesPage() { | ||
| const [classId, setClassId] = useQueryState("classId", parseAsString); | ||
|
|
||
| return ( | ||
| <PageLayout> | ||
| <ClassListView /> | ||
| <PageLayout | ||
| open={!!classId} | ||
| onOpenChange={(open) => { | ||
| if (!open) setClassId(null); | ||
| }} | ||
| > | ||
| <ClassListView classId={classId} setClassId={setClassId} /> | ||
| </PageLayout> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,13 +18,16 @@ import { WithPermission } from "@/components/utils/with-permission"; | |
| import { CoverageRequestCategory, CoverageStatus } from "@/models/api/coverage"; | ||
| import { useAuth } from "@/providers/client-auth-provider"; | ||
| import { ChevronLeft, ChevronRight } from "lucide-react"; | ||
| import { useCoveragePage } from "./coverage-page-context"; | ||
| import { | ||
| useCoveragePage, | ||
| type CoverageListItem, | ||
| } from "./coverage-page-context"; | ||
| import { FillCoverageButton } from "@/components/coverage/primitives/fill-coverage-button"; | ||
| import { WithdrawCoverageButton } from "@/components/coverage/primitives/withdraw-coverage-button"; | ||
| import { useEffect } from "react"; | ||
| import { Separator } from "@/components/ui/separator"; | ||
| import { UserList } from "@/components/users/user-list"; | ||
| import { Button } from "@/components/ui/button"; | ||
| import { clientApi } from "@/trpc/client"; | ||
|
|
||
| const dateFormatter = new Intl.DateTimeFormat("en-US", { | ||
| month: "long", | ||
|
|
@@ -39,12 +42,19 @@ const timeFormatter = new Intl.DateTimeFormat("en-US", { | |
| }); | ||
|
|
||
| export function CoverageAside() { | ||
| const { selectedItem, closeAside, goToNext, goToPrev } = useCoveragePage(); | ||
| const { selectedCoverageId, goToNext, goToPrev, hasNext, hasPrev } = | ||
| useCoveragePage(); | ||
| const { user } = useAuth(); | ||
|
|
||
| useEffect(() => { | ||
| if (!selectedItem) closeAside(); | ||
| }, [selectedItem, closeAside]); | ||
| const { data } = clientApi.coverage.byId.useQuery( | ||
| { coverageRequestId: selectedCoverageId ?? "" }, | ||
| { | ||
| enabled: !!selectedCoverageId, | ||
| suspense: true, | ||
| meta: { suppressToast: true }, | ||
| }, | ||
| ); | ||
greptile-apps[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const selectedItem = data as CoverageListItem | undefined; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsafe type cast may hide server/client type mismatch
Remove the cast and fix any TypeScript errors surfaced by the raw |
||
|
|
||
| if (!selectedItem) return null; | ||
|
|
||
|
|
@@ -208,10 +218,10 @@ export function CoverageAside() { | |
|
|
||
| <AsideFooter> | ||
| <div className="flex gap-2"> | ||
| <Button variant="ghost" onClick={() => goToPrev()}> | ||
| <Button variant="ghost" disabled={!hasPrev} onClick={() => goToPrev()}> | ||
| <ChevronLeft></ChevronLeft> | ||
| </Button> | ||
| <Button variant="ghost" onClick={() => goToNext()}> | ||
| <Button variant="ghost" disabled={!hasNext} onClick={() => goToNext()}> | ||
| <ChevronRight></ChevronRight> | ||
| </Button> | ||
| </div> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side effect inside a
setStateupdater functionsetQueryTerm(linkedClass.termId)is called inside thesetSelectedTermIdupdater function. React's updater functions are expected to be pure — they must not produce side effects. In React Strict Mode (enabled by default in development), React intentionally invokes updaters twice to surface these violations; this will causesetQueryTerm(and its URL mutation) to fire twice on every effect run.Perform both state mutations directly instead of nesting one inside the other: