Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added feature: Header auto hide and unhide as user scroll. #363

Closed
wants to merge 4 commits into from
Closed

added feature: Header auto hide and unhide as user scroll. #363

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
032dab3
added feature: Header auto hide and unhide as user scroll. #361
HusainModiwala Apr 11, 2024
b36ce55
removed useCallback hook as it's not needed and was causing an infini…
HusainModiwala Apr 11, 2024
acf9c8c
small cleanup
HusainModiwala Apr 11, 2024
83753a7
fixed linting
HusainModiwala Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion src/components/Appbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,47 @@ import { ThemeToggler } from './ThemeToggler';
import { NavigationMenu } from './landing/appbar/nav-menu';
import SearchBar from './search/SearchBar';
import MobileScreenSearch from './search/MobileScreenSearch';
import { useEffect, useState } from 'react';

export const Appbar = () => {
const session = useSession();
const [sidebarOpen, setSidebarOpen] = useRecoilState(sidebarOpenAtom);
const [show, setShow] = useState(true);
const [lastScrollY, setLastScrollY] = useState(0);
const currentPath = usePathname();
const params = useParams();
let bookmarkPageUrl = null;
if (params.courseId && params.courseId[0]) {
bookmarkPageUrl = `/courses/${params.courseId[0]}/bookmarks`;
}

const controlAppbar = () => {
if (typeof window !== 'undefined') {
if (window.scrollY > lastScrollY) { // if scroll down hide the navbar
setShow(false);
} else { // if scroll up show the navbar
setShow(true);
}

// remember current page location to use in the next move
setLastScrollY(window.scrollY);
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this not causing an infinite loop?
You're updating the lastScrollY here and have it as a dependency to the effect?
Is it because it's the same the second time?
Doesn't matter but we should confirm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes you are correct it is causing an issue. Actually I had added the useCallback after I was done testing the change. And then forgot to test again. Will correct and update. Sorry for the inconvinience.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still dont see a useCallback here?
This will cause the effect to run on every render

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but using useCallback was causing infinite loop and it was not rendering that's why I removed that.

}
};

useEffect(() => {
if (typeof window !== 'undefined') {
window.addEventListener('scroll', controlAppbar);

// cleanup function
return () => {
window.removeEventListener('scroll', controlAppbar);
};
}
}, [controlAppbar]);

return (
<>
<nav className="fixed z-50 top-0 px-4 w-full h-16 border-b shadow-sm bg-background/80 backdrop-blur-md flex items-center gap-2 print:hidden">
<nav className={`fixed z-50 top-0 px-4 w-full h-16 border-b shadow-sm bg-background/80 backdrop-blur-md flex items-center gap-2 print:hidden transform ${show ? 'translate-y-0' : '-translate-y-full'}`}>
{currentPath.includes('courses') && (
<ToggleButton
onClick={() => {
Expand Down
Loading