Skip to content

Commit 2eb0a35

Browse files
fix : Mobile navigation links scroll to section then immediately return to top (#453)
* fix : mobile-navbar Signed-off-by: greedy-wudpeckr <mudituiet@gmail.com> * added tests for navbar-mobile Signed-off-by: greedy-wudpeckr <mudituiet@gmail.com> --------- Signed-off-by: greedy-wudpeckr <mudituiet@gmail.com>
1 parent 538523b commit 2eb0a35

2 files changed

Lines changed: 129 additions & 1 deletion

File tree

frontend/src/components/HomeComponents/Navbar/NavbarMobile.tsx

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,23 @@ export const NavbarMobile = (
106106
props.setIsOpen(false);
107107
};
108108

109+
const handleNavClick = (
110+
e: React.MouseEvent<HTMLAnchorElement>,
111+
href: string
112+
) => {
113+
e.preventDefault();
114+
props.setIsOpen(false);
115+
116+
// Wait for sheet to close, then scroll
117+
setTimeout(() => {
118+
const targetId = href.replace('#', '');
119+
const element = document.getElementById(targetId);
120+
if (element) {
121+
element.scrollIntoView({ behavior: 'smooth', block: 'start' });
122+
}
123+
}, 300);
124+
};
125+
109126
return (
110127
<span className="flex md:hidden">
111128
<ModeToggle />
@@ -132,7 +149,7 @@ export const NavbarMobile = (
132149
rel="noreferrer noopener"
133150
key={label}
134151
href={href}
135-
onClick={() => props.setIsOpen(false)}
152+
onClick={(e) => handleNavClick(e, href)}
136153
className={buttonVariants({ variant: 'ghost' })}
137154
>
138155
{label}

frontend/src/components/HomeComponents/Navbar/__tests__/NavbarMobile.test.tsx

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,117 @@ describe('NavbarMobile', () => {
151151

152152
expect(screen.getByText(/Enable Auto-Sync/i)).toBeInTheDocument();
153153
});
154+
155+
describe('Navigation link click behavior', () => {
156+
beforeEach(() => {
157+
// Mock scrollIntoView
158+
Element.prototype.scrollIntoView = jest.fn();
159+
// Mock setTimeout
160+
jest.useFakeTimers();
161+
});
162+
163+
afterEach(() => {
164+
jest.runOnlyPendingTimers();
165+
jest.useRealTimers();
166+
});
167+
168+
it('should close the sheet immediately when clicking a nav link', () => {
169+
render(<NavbarMobile {...openProps} />);
170+
171+
const tasksLink = screen.getByText('Tasks');
172+
fireEvent.click(tasksLink);
173+
174+
expect(mockSetIsOpen).toHaveBeenCalledWith(false);
175+
});
176+
177+
it('should scroll to target element after 300ms delay', () => {
178+
// Create a mock element
179+
const mockElement = document.createElement('div');
180+
mockElement.id = 'tasks';
181+
document.body.appendChild(mockElement);
182+
mockElement.scrollIntoView = jest.fn();
183+
184+
render(<NavbarMobile {...openProps} />);
185+
186+
const tasksLink = screen.getByText('Tasks');
187+
fireEvent.click(tasksLink);
188+
189+
// scrollIntoView should not be called immediately
190+
expect(mockElement.scrollIntoView).not.toHaveBeenCalled();
191+
192+
// Fast-forward time by 300ms
193+
jest.advanceTimersByTime(300);
194+
195+
// Now scrollIntoView should have been called
196+
expect(mockElement.scrollIntoView).toHaveBeenCalledWith({
197+
behavior: 'smooth',
198+
block: 'start',
199+
});
200+
201+
// Cleanup
202+
document.body.removeChild(mockElement);
203+
});
204+
205+
it('should extract the correct target ID from href', () => {
206+
const mockElement = document.createElement('div');
207+
mockElement.id = 'setup-guide';
208+
document.body.appendChild(mockElement);
209+
mockElement.scrollIntoView = jest.fn();
210+
211+
render(<NavbarMobile {...openProps} />);
212+
213+
const setupGuideLink = screen.getByText('Setup Guide');
214+
fireEvent.click(setupGuideLink);
215+
216+
jest.advanceTimersByTime(300);
217+
218+
expect(mockElement.scrollIntoView).toHaveBeenCalledWith({
219+
behavior: 'smooth',
220+
block: 'start',
221+
});
222+
223+
document.body.removeChild(mockElement);
224+
});
225+
226+
it('should handle case when target element does not exist', () => {
227+
render(<NavbarMobile {...openProps} />);
228+
229+
const homeLink = screen.getByText('Home');
230+
fireEvent.click(homeLink);
231+
232+
// Should not throw an error even if element doesn't exist
233+
expect(() => {
234+
jest.advanceTimersByTime(300);
235+
}).not.toThrow();
236+
});
237+
238+
it('should handle multiple rapid clicks correctly', () => {
239+
const mockElement = document.createElement('div');
240+
mockElement.id = 'tasks';
241+
document.body.appendChild(mockElement);
242+
mockElement.scrollIntoView = jest.fn();
243+
244+
render(<NavbarMobile {...openProps} />);
245+
246+
const tasksLink = screen.getByText('Tasks');
247+
248+
// Click multiple times
249+
fireEvent.click(tasksLink);
250+
fireEvent.click(tasksLink);
251+
fireEvent.click(tasksLink);
252+
253+
// Sheet should be closed on each click
254+
expect(mockSetIsOpen).toHaveBeenCalledTimes(3);
255+
expect(mockSetIsOpen).toHaveBeenCalledWith(false);
256+
257+
jest.advanceTimersByTime(300);
258+
259+
// scrollIntoView should be called for each click
260+
expect(mockElement.scrollIntoView).toHaveBeenCalledTimes(3);
261+
262+
document.body.removeChild(mockElement);
263+
});
264+
});
154265
});
155266

156267
describe('NavbarMobile component using snapshot', () => {

0 commit comments

Comments
 (0)