Copilot commented on code in PR #9355:
URL: https://github.com/apache/gravitino/pull/9355#discussion_r2580733255
##########
web/web/src/lib/provider/session.js:
##########
@@ -66,17 +66,16 @@ const AuthProvider = ({ children }) => {
const goToMetalakeListPage = () => {
try {
- const pathname = window.location.pathname
- const doneFlag = sessionStorage.getItem('authInitDone')
+ let pathname = window.location.pathname
- if (!doneFlag) {
- sessionStorage.setItem('authInitDone', 'true')
-
- if (pathname === '/' || pathname === '/ui' || pathname === '/ui/') {
- router.replace('/metalakes')
- } else {
- router.replace(pathname + window.location.search)
- }
+ // Remove /ui prefix since Next.js basePath will add it automatically
+ if (pathname.startsWith('/ui')) {
+ pathname = pathname.slice(3) || '/'
+ }
+ if (pathname === '/' || pathname === '') {
+ router.push('/metalakes')
+ } else {
+ router.push(pathname + window.location.search)
Review Comment:
[nitpick] Changing from `router.replace` to `router.push` alters the browser
history behavior. With `router.push`, each authentication initialization will
add a new entry to the browser's history stack, which means users will need to
press "back" multiple times to navigate away from the application after
multiple page reloads.
While this change fixes the redirect issue described in the PR, consider
whether this is the desired behavior. If the intent is to avoid adding to
history, you might want to investigate why `router.replace` wasn't working
under the same session and consider alternative solutions that preserve the
original behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
