-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Cleanup : Remove HistoryAPIProvider and useAppHistory and rely on browser history to prevent navigation loops #16114
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: develop
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
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,31 @@ | ||
| import { Link } from "raviger"; | ||
|
|
||
| import { Button } from "@/components/ui/button"; | ||
|
|
||
| import useAppHistory from "@/hooks/useAppHistory"; | ||
| import { Button, ButtonSize, ButtonVariant } from "@/components/ui/button"; | ||
|
|
||
| type BackButtonProps = { | ||
| to?: string; | ||
| fallbackUrl?: string; | ||
| } & React.ComponentProps<typeof Button>; | ||
|
|
||
| children: React.ReactNode; | ||
| variant?: ButtonVariant; | ||
| className?: string; | ||
| size?: ButtonSize; | ||
| disabled?: boolean; | ||
| } & Omit<React.ComponentProps<"button">, "onClick">; | ||
| export default function BackButton({ | ||
| to, | ||
| fallbackUrl, | ||
| children, | ||
| variant = "outline", | ||
| className, | ||
| size = "default", | ||
| disabled = false, | ||
| ...props | ||
| }: BackButtonProps) { | ||
| const { history } = useAppHistory(); | ||
|
|
||
| to ??= history[1] ?? fallbackUrl; | ||
|
|
||
| if (!to) { | ||
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <Button variant="outline" data-shortcut-id="go-back" asChild {...props}> | ||
| <Link basePath="/" href={to}> | ||
| {props.children} | ||
| </Link> | ||
| <Button | ||
| type="button" | ||
| variant={variant} | ||
| onClick={() => history.back()} | ||
| className={className} | ||
| size={size} | ||
| disabled={disabled} | ||
| {...props} | ||
|
Comment on lines
+19
to
+26
Contributor
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. Prop spread order allows The 🛡️ Proposed fix: spread props before explicit values or omit `type`Option 1 - Spread props first so explicit values take precedence: <Button
+ {...props}
type="button"
variant={variant}
onClick={() => history.back()}
className={className}
size={size}
disabled={disabled}
- {...props}
>Option 2 - Omit -} & Omit<React.ComponentProps<"button">, "onClick">;
+} & Omit<React.ComponentProps<"button">, "onClick" | "type">;🤖 Prompt for AI Agents
Comment on lines
+19
to
+26
Comment on lines
+20
to
+26
Comment on lines
+19
to
+26
|
||
| > | ||
|
Comment on lines
+19
to
+27
|
||
| {children} | ||
| </Button> | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,7 @@ import { cn } from "@/lib/utils"; | |||||||||||||||||
|
|
||||||||||||||||||
| import { Button } from "@/components/ui/button"; | ||||||||||||||||||
|
|
||||||||||||||||||
| import useAppHistory from "@/hooks/useAppHistory"; | ||||||||||||||||||
| import { navigate } from "raviger"; | ||||||||||||||||||
|
|
||||||||||||||||||
| type ErrorType = "PAGE_NOT_FOUND" | "PAGE_LOAD_ERROR" | "CUSTOM_ERROR"; | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -32,7 +32,6 @@ export default function ErrorPage({ | |||||||||||||||||
| ...props | ||||||||||||||||||
| }: ErrorPageProps) { | ||||||||||||||||||
| const { t } = useTranslation(); | ||||||||||||||||||
| const { goBack } = useAppHistory(); | ||||||||||||||||||
| const [mousePos, setMousePos] = useState({ x: 0, y: 0 }); | ||||||||||||||||||
| const containerRef = useRef<HTMLDivElement>(null); | ||||||||||||||||||
| const [isHovering, setIsHovering] = useState(false); | ||||||||||||||||||
|
|
@@ -176,7 +175,7 @@ export default function ErrorPage({ | |||||||||||||||||
| <div className="mt-6"> | ||||||||||||||||||
| <Button | ||||||||||||||||||
| onClick={() => { | ||||||||||||||||||
| goBack("/"); | ||||||||||||||||||
| navigate("/"); | ||||||||||||||||||
| window.location.reload(); | ||||||||||||||||||
| }} | ||||||||||||||||||
|
Comment on lines
177
to
180
Contributor
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. 🧩 Analysis chain🏁 Script executed: cat -n src/components/ErrorPages/DefaultErrorPage.tsx | head -200Repository: ohcnetwork/care_fe Length of output: 7533 🏁 Script executed: rg -A 5 -B 5 "onClick.*navigate.*reload" src/components/ErrorPages/Repository: ohcnetwork/care_fe Length of output: 44 🏁 Script executed: rg -A 3 -B 3 "window.location.replace" src/Repository: ohcnetwork/care_fe Length of output: 44 🏁 Script executed: rg -B 2 -A 5 "navigate\(" src/components/ErrorPages/ | head -30Repository: ohcnetwork/care_fe Length of output: 831 🏁 Script executed: rg "useRedirect|navigate.*replace" src/ -A 2 -B 2 | head -40Repository: ohcnetwork/care_fe Length of output: 899 🏁 Script executed: fd -e "ts" -e "tsx" src/ -x grep -l "raviger" | head -5 | xargs -I {} sh -c 'echo "File: {}"; grep -A 2 -B 2 "navigate\(" {} | head -15'Repository: ohcnetwork/care_fe Length of output: 383 🏁 Script executed: rg "PAGE_LOAD_ERROR|PAGE_NOT_FOUND" src/ -A 5 -B 5 | head -50Repository: ohcnetwork/care_fe Length of output: 3185 🏁 Script executed: rg "window.location.reload" src/ -B 3 -A 1Repository: ohcnetwork/care_fe Length of output: 996 Simplify recovery navigation to avoid redundant calls. The 💡 Suggested fix onClick={() => {
- navigate("/");
- window.location.reload();
+ navigate("/", { replace: true });
}}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| className="rounded-md bg-primary-700 px-3.5 py-2.5 text-sm font-semibold text-white shadow-sm hover:bg-primary-600 focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-800" | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { useQuery } from "@tanstack/react-query"; | ||
| import { useQueryParams } from "raviger"; | ||
| import { navigate, useQueryParams } from "raviger"; | ||
| import { useEffect } from "react"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { toast } from "sonner"; | ||
|
|
@@ -14,8 +14,6 @@ import CreateEncounterForm from "@/components/Encounter/CreateEncounterForm"; | |
| import { TimelineEncounterCard } from "@/components/Facility/EncounterCard"; | ||
| import { PatientProps } from "@/components/Patient/PatientDetailsTab"; | ||
|
|
||
| import useAppHistory from "@/hooks/useAppHistory"; | ||
|
|
||
| import { getPermissions } from "@/common/Permissions"; | ||
|
|
||
| import { ShortcutBadge } from "@/Utils/keyboardShortcutComponents"; | ||
|
|
@@ -40,7 +38,6 @@ const EncounterHistory = (props: PatientProps) => { | |
| hasPermission, | ||
| patientData.permissions, | ||
| ); | ||
| const { goBack } = useAppHistory(); | ||
|
|
||
| const { data: encounterData, isLoading } = useQuery({ | ||
| queryKey: ["encounterHistory", patientId, qParams], | ||
|
|
@@ -57,7 +54,7 @@ const EncounterHistory = (props: PatientProps) => { | |
| useEffect(() => { | ||
| if (!canViewPatients) { | ||
| toast.error(t("no_permission_to_view_page")); | ||
| goBack(`/facility/${facilityId}/patient/${patientId}`); | ||
| navigate(`/facility/${facilityId}/patient/${patientId}`); | ||
|
Contributor
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. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify whether PatientProps allows missing facilityId and which patient routes exist.
echo "== PatientProps definition =="
fd "PatientDetailsTab.tsx" src | xargs -I{} rg -n -C3 "interface PatientProps|facilityId\\??|facilityId:" {}
echo
echo "== Patient history route patterns in router files =="
fd ".*Routes\\.tsx$" src/Routers/routes | xargs -I{} rg -n -C2 "/facility/:facilityId/patient/:patientId|/patient/:patientId" {}Repository: ohcnetwork/care_fe Length of output: 7397 🏁 Script executed: #!/bin/bash
# Locate and read EncounterHistory.tsx
echo "== Finding EncounterHistory.tsx =="
fd "EncounterHistory.tsx" src/
echo
echo "== Reading EncounterHistory.tsx (full file) =="
fd "EncounterHistory.tsx" src/ | head -1 | xargs catRepository: ohcnetwork/care_fe Length of output: 5700 🏁 Script executed: #!/bin/bash
# Search for PatientProps interface definition
echo "== PatientProps definition =="
rg -n "interface PatientProps|type PatientProps" src/components/Patient/PatientDetailsTab/ -A10
echo
echo "== facilityId usage in EncounterHistory context =="
rg -n "facilityId" src/components/Patient/PatientDetailsTab/EncounterHistory.tsx -B2 -A2Repository: ohcnetwork/care_fe Length of output: 2049 🏁 Script executed: #!/bin/bash
# Verify the patient-scoped route exists and is used in ClinicalHistoryPage
echo "== Search for /patient/:patientId/history pattern =="
rg "/patient/:patientId/history" src/Routers/routes -B1 -A3
echo
echo "== Verify ClinicalHistoryPage accepts patientId without facilityId =="
rg -n "ClinicalHistoryPage" src/Routers/routes -B2 -A8 | head -30Repository: ohcnetwork/care_fe Length of output: 4385 Guard unauthorized redirect to handle missing Line 57 unconditionally navigates to a facility-scoped URL, but 💡 Suggested fix- navigate(`/facility/${facilityId}/patient/${patientId}`);
+ navigate(
+ facilityId
+ ? `/facility/${facilityId}/patient/${patientId}`
+ : `/patient/${patientId}`,
+ );🤖 Prompt for AI Agents |
||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [canViewPatients]); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.