From bc57fde0a3cafb64906a19efd52a5cd5c798f69f Mon Sep 17 00:00:00 2001 From: doubleface Date: Thu, 9 Apr 2026 09:06:24 +0200 Subject: [PATCH 1/2] fix: Reset lastClicked in NavLink when route matches Also added unit tests for these cases --- src/modules/navigation/NavLink.jsx | 11 +- src/modules/navigation/NavLink.spec.jsx | 161 ++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 src/modules/navigation/NavLink.spec.jsx diff --git a/src/modules/navigation/NavLink.jsx b/src/modules/navigation/NavLink.jsx index cddcf792b9..809f6fec0c 100644 --- a/src/modules/navigation/NavLink.jsx +++ b/src/modules/navigation/NavLink.jsx @@ -1,6 +1,6 @@ import cx from 'classnames' import PropTypes from 'prop-types' -import React from 'react' +import React, { useEffect } from 'react' import { useLocation } from 'react-router-dom' import { NavLink as UINavLink } from 'cozy-ui/transpiled/react/Nav' @@ -19,6 +19,15 @@ const NavLink = ({ clickState: [lastClicked, setLastClicked] }) => { const location = useLocation() + + useEffect(() => { + if (!lastClicked) return + + if (navLinkMatch(rx, lastClicked, location.pathname)) { + setLastClicked(null) + } + }, [location.pathname, rx, lastClicked, setLastClicked]) + const pathname = lastClicked ? lastClicked : location.pathname const isActive = navLinkMatch(rx, to, pathname) return ( diff --git a/src/modules/navigation/NavLink.spec.jsx b/src/modules/navigation/NavLink.spec.jsx new file mode 100644 index 0000000000..54812f57c7 --- /dev/null +++ b/src/modules/navigation/NavLink.spec.jsx @@ -0,0 +1,161 @@ +import { render, fireEvent, act } from '@testing-library/react' +import React from 'react' +import { useLocation } from 'react-router-dom' + +import { NavLink } from './NavLink' + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useLocation: jest.fn() +})) + +jest.mock('cozy-ui/transpiled/react/Nav', () => ({ + NavLink: { className: 'nav-link', activeClassName: 'active' } +})) + +describe('NavLink', () => { + beforeEach(() => { + useLocation.mockReturnValue({ pathname: '/recent' }) + }) + + describe('click handler', () => { + it('calls setLastClicked with to on click', () => { + const setLastClicked = jest.fn() + const { getByRole } = render( + + Drive + + ) + fireEvent.click(getByRole('link')) + expect(setLastClicked).toHaveBeenCalledWith('/folder') + }) + + it('prevents default when no to prop', () => { + const setLastClicked = jest.fn() + const { getByRole } = render( + Drive + ) + const event = new MouseEvent('click', { bubbles: true, cancelable: true }) + getByRole('link').dispatchEvent(event) + expect(event.defaultPrevented).toBe(true) + }) + }) + + describe('optimistic active state', () => { + it('shows as active when lastClicked matches rx', () => { + const rx = /\/(folder|nextcloud)(\/.*)?/ + const { getByRole } = render( + + Drive + + ) + expect(getByRole('link').className).toContain('active') + }) + + it('is not active when lastClicked does not match rx', () => { + const rx = /\/(folder|nextcloud)(\/.*)?/ + const { getByRole } = render( + + Drive + + ) + expect(getByRole('link').className).not.toContain('active') + }) + }) + + describe('useEffect: reset lastClicked when route arrives', () => { + it('does not reset lastClicked while the route has not yet changed', () => { + const setLastClicked = jest.fn() + useLocation.mockReturnValue({ pathname: '/recent' }) + + render( + + Drive + + ) + + expect(setLastClicked).not.toHaveBeenCalledWith(null) + }) + + it('resets lastClicked once the route matches rx', async () => { + const setLastClicked = jest.fn() + useLocation.mockReturnValue({ pathname: '/recent' }) + const rx = /\/(folder|nextcloud)(\/.*)?/ + + const { rerender } = render( + + Drive + + ) + + useLocation.mockReturnValue({ pathname: '/folder' }) + await act(async () => { + rerender( + + Drive + + ) + }) + + expect(setLastClicked).toHaveBeenCalledWith(null) + }) + + it('resets lastClicked once the route matches without rx', async () => { + const setLastClicked = jest.fn() + useLocation.mockReturnValue({ pathname: '/recent' }) + + const { rerender } = render( + + Drive + + ) + + useLocation.mockReturnValue({ pathname: '/folder' }) + await act(async () => { + rerender( + + Drive + + ) + }) + + expect(setLastClicked).toHaveBeenCalledWith(null) + }) + + it('does not reset lastClicked when route changes to a different destination', async () => { + const setLastClicked = jest.fn() + useLocation.mockReturnValue({ pathname: '/recent' }) + const rx = /\/(folder|nextcloud)(\/.*)?/ + + const { rerender } = render( + + Drive + + ) + + // Route changed but to /trash, not /folder + useLocation.mockReturnValue({ pathname: '/trash' }) + await act(async () => { + rerender( + + Drive + + ) + }) + + expect(setLastClicked).not.toHaveBeenCalledWith(null) + }) + }) +}) From d672a3a0622c1b071f0fd2dde03315be3fbcd4a3 Mon Sep 17 00:00:00 2001 From: doubleface Date: Thu, 9 Apr 2026 18:14:46 +0200 Subject: [PATCH 2/2] fix: Reset lastClicked in NavLink on back button navigation --- src/modules/navigation/NavLink.jsx | 10 ++++++++-- src/modules/navigation/NavLink.spec.jsx | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/modules/navigation/NavLink.jsx b/src/modules/navigation/NavLink.jsx index 809f6fec0c..a95b20bf86 100644 --- a/src/modules/navigation/NavLink.jsx +++ b/src/modules/navigation/NavLink.jsx @@ -1,6 +1,6 @@ import cx from 'classnames' import PropTypes from 'prop-types' -import React, { useEffect } from 'react' +import React, { useEffect, useRef } from 'react' import { useLocation } from 'react-router-dom' import { NavLink as UINavLink } from 'cozy-ui/transpiled/react/Nav' @@ -19,12 +19,18 @@ const NavLink = ({ clickState: [lastClicked, setLastClicked] }) => { const location = useLocation() + const prevPathnameRef = useRef(location.pathname) useEffect(() => { + const prevPathname = prevPathnameRef.current + prevPathnameRef.current = location.pathname + if (!lastClicked) return if (navLinkMatch(rx, lastClicked, location.pathname)) { - setLastClicked(null) + setLastClicked(null) // route arrived at destination + } else if (location.pathname !== prevPathname) { + setLastClicked(null) // route changed but went elsewhere (e.g. back button) } }, [location.pathname, rx, lastClicked, setLastClicked]) diff --git a/src/modules/navigation/NavLink.spec.jsx b/src/modules/navigation/NavLink.spec.jsx index 54812f57c7..38035e6327 100644 --- a/src/modules/navigation/NavLink.spec.jsx +++ b/src/modules/navigation/NavLink.spec.jsx @@ -130,7 +130,7 @@ describe('NavLink', () => { expect(setLastClicked).toHaveBeenCalledWith(null) }) - it('does not reset lastClicked when route changes to a different destination', async () => { + it('resets lastClicked when route changes to a different destination (e.g. back button)', async () => { const setLastClicked = jest.fn() useLocation.mockReturnValue({ pathname: '/recent' }) const rx = /\/(folder|nextcloud)(\/.*)?/ @@ -141,7 +141,7 @@ describe('NavLink', () => { ) - // Route changed but to /trash, not /folder + // Route changed but to /trash, not /folder (e.g. back button or unexpected navigation) useLocation.mockReturnValue({ pathname: '/trash' }) await act(async () => { rerender( @@ -155,7 +155,7 @@ describe('NavLink', () => { ) }) - expect(setLastClicked).not.toHaveBeenCalledWith(null) + expect(setLastClicked).toHaveBeenCalledWith(null) }) }) })