Skip to content

Fix sidebar active route sync when clicking Flowise logo#6580

Open
cgb102000 wants to merge 1 commit into
FlowiseAI:mainfrom
cgb102000:fix/sidebar-active-route-sync
Open

Fix sidebar active route sync when clicking Flowise logo#6580
cgb102000 wants to merge 1 commit into
FlowiseAI:mainfrom
cgb102000:fix/sidebar-active-route-sync

Conversation

@cgb102000

Copy link
Copy Markdown

This PR fixes a sidebar navigation bug where clicking the Flowise logo navigated to /chatflows but did not update the active menu state in the sidebar.

What changed:

Updated index.jsx
Added route tracking using useLocation()
Synced sidebar active state on every route change
Ensured root path / defaults to chatflows
Why it matters:

Prevents the sidebar from showing the wrong selected item after logo navigation
Keeps customization.isOpen aligned with the current URL
Improves navigation consistency across the UI
How to test:

Start the UI app
Click the Flowise logo
Confirm the Chatflows menu item becomes active
Navigate to other menu pages and verify the sidebar selection follows the route

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the NavItem component to use useLocation from react-router-dom instead of accessing document.location.pathname directly, and updates the useEffect hook's dependency array. However, including itemHandler in the dependency array triggers an infinite loop when navigating to the root path (/) due to recreation of the function on every render and subsequent Redux state updates. A fix has been suggested to simplify the logic and remove itemHandler from the dependencies.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 82 to +96
useEffect(() => {
if (navType === 'MENU') {
const currentIndex = document.location.pathname
.toString()
.split('/')
.findIndex((id) => id === item.id)
if (currentIndex > -1) {
dispatch({ type: MENU_OPEN, id: item.id })
}
if (!document.location.pathname.toString().split('/')[1]) {
itemHandler('chatflows')
}
if (navType !== 'MENU') return

const pathnameParts = location.pathname.toString().split('/')
const activePathSegment = pathnameParts[1]

if (!activePathSegment) {
itemHandler('chatflows')
return
}

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [navType])
if (activePathSegment === item.id) {
dispatch({ type: MENU_OPEN, id: item.id })
}
}, [dispatch, item.id, location.pathname, navType, itemHandler])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

An infinite loop is triggered when navigating to the root path (/).

Cause

  1. itemHandler is a standard function recreated on every render.
  2. Since itemHandler is included in the useEffect dependency array, any re-render triggers the effect.
  3. When on the root path (/), !activePathSegment is true, so itemHandler('chatflows') is called.
  4. This dispatches Redux actions, updating the Redux store.
  5. The Redux state update triggers a re-render of NavItem (via useSelector), recreating itemHandler and triggering the useEffect again, resulting in an infinite loop.

Solution

We can simplify the logic by defaulting activePathSegment to 'chatflows' when it is empty, and only dispatching MENU_OPEN when activePathSegment === item.id. This completely removes the need to call itemHandler inside useEffect, removing it from the dependency array and eliminating the infinite loop entirely. It also avoids redundant dispatches from other NavItem components.

    useEffect(() => {
        if (navType !== 'MENU') return

        const pathnameParts = location.pathname.toString().split('/')
        const activePathSegment = pathnameParts[1] || 'chatflows'

        if (activePathSegment === item.id) {
            dispatch({ type: MENU_OPEN, id: item.id })
        }
    }, [dispatch, item.id, location.pathname, navType])

@cgb102000

Copy link
Copy Markdown
Author

I applied the review suggestion: the effect now defaults the active route segment to chatflows for /, and dispatches MENU_OPEN only when the item id matches. This removes itemHandler from the dependency array and fixes the loop risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant