feat: Add Discard popup for browser navigation#4552
feat: Add Discard popup for browser navigation#4552chriskari merged 12 commits intokyma-project:mainfrom
Conversation
OliwiaGowor
requested review from
KonradPietocha,
akucharska,
awoloszyn22,
chriskari,
dbadura and
mrCherry97
as code owners
| const blocker = useBlocker(({ historyAction }) => { | ||
| const isBrowserNav = historyAction === 'POP'; | ||
|
|
||
| return ( | ||
| isBrowserNav && isResourceEdited.isEdited && formOpen && webNavBlocker | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Every call site of useFormNavigation (about 10+ components) registers a useBlocker with React Router, even though webNavBlocker defaults to false and some of these blockers never actually block. This is relatively harmless but still creates unnecessary overhead.
instead we can implement it like this
const blocker = useBlocker( webNavBlocker ? ({ historyAction }) => { const isBrowserNav = historyAction === 'POP'; return isBrowserNav && isResourceEdited.isEdited && formOpen; } : false, );
| const router = useMemo( | ||
| () => | ||
| createBrowserRouter([ | ||
| { | ||
| path: '*', | ||
| element: ( | ||
| <Suspense fallback={<Spinner />}> | ||
| <DynamicPageComponent | ||
| {...props} | ||
| inlineEditForm={ | ||
| props?.inlineEditForm ? transformedForm : undefined | ||
| } | ||
| /> | ||
| </Suspense> | ||
| ), | ||
| }, | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| ]), | ||
| [props], | ||
| ); |
There was a problem hiding this comment.
This is problematic, useMemo(..., [props]) recreates the router every render because props is a new object reference each time. Recreating the router resets all internal router state (history, blockers, etc.).
2 participants
Description
Changes proposed in this pull request:
<BrowserRouter>tocreateBrowserRouterand<RouterProvider>Related issue(s)
Closes #4319
Definition of done
backlog#4567