Skip to content

Fixes #6693: fixes search functionality on iOS devices #6743

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

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Fixes #6693: fixes search functionality on iOS devices #6743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39c0442
change isOpen variable in TopNav component to isMenuOpen to different…
ramizackaryshamir Apr 9, 2024
5011e26
move initialization of ShowSearch state to top-level of TopNav component
ramizackaryshamir Apr 9, 2024
1b10d2d
remove initialYScroll prop from DocSearchModal component to fix searc…
ramizackaryshamir Apr 9, 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
32 changes: 16 additions & 16 deletions src/components/Layout/TopNav/TopNav.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,11 @@ export default function TopNav({
breadcrumbs: RouteItem[];
section: 'learn' | 'reference' | 'community' | 'blog' | 'home' | 'unknown';
}) {
const [isOpen, setIsOpen] = useState(false);
const [isMenuOpen, setIsMenuOpen] = useState(false);
const [showSearch, setShowSearch] = useState(false);
const [isScrolled, setIsScrolled] = useState(false);
const scrollParentRef = useRef<HTMLDivElement>(null);
const {asPath} = useRouter();
const [isScrolled, setIsScrolled] = useState(false);

// HACK. Fix up the data structures instead.
if ((routeTree as any).routes.length === 1) {
Expand All @@ -154,18 +155,18 @@ export default function TopNav({

// While the overlay is open, disable body scroll.
useEffect(() => {
if (isOpen) {
if (isMenuOpen) {
const preferredScrollParent = scrollParentRef.current!;
disableBodyScroll(preferredScrollParent);
return () => enableBodyScroll(preferredScrollParent);
} else {
return undefined;
}
}, [isOpen]);
}, [isMenuOpen]);

// Close the overlay on any navigation.
useEffect(() => {
setIsOpen(false);
setIsMenuOpen(false);
}, [asPath]);

// Also close the overlay if the window gets resized past mobile layout.
Expand All @@ -175,7 +176,7 @@ export default function TopNav({

function closeIfNeeded() {
if (!media.matches) {
setIsOpen(false);
setIsMenuOpen(false);
}
}

Expand Down Expand Up @@ -204,7 +205,6 @@ export default function TopNav({
return () => observer.disconnect();
}, []);

const [showSearch, setShowSearch] = useState(false);
const onOpenSearch = useCallback(() => {
startTransition(() => {
setShowSearch(true);
Expand All @@ -224,28 +224,28 @@ export default function TopNav({
<div ref={scrollDetectorRef} />
<div
className={cn(
isOpen
isMenuOpen
? 'h-screen sticky top-0 lg:bottom-0 lg:h-screen flex flex-col shadow-nav dark:shadow-nav-dark z-20'
: 'z-50 sticky top-0'
)}>
<nav
className={cn(
'duration-300 backdrop-filter backdrop-blur-lg backdrop-saturate-200 transition-shadow bg-opacity-90 items-center w-full flex justify-between bg-wash dark:bg-wash-dark dark:bg-opacity-95 px-1.5 lg:pe-5 lg:ps-4 z-50',
{'dark:shadow-nav-dark shadow-nav': isScrolled || isOpen}
{'dark:shadow-nav-dark shadow-nav': isScrolled || isMenuOpen}
)}>
<div className="flex items-center justify-between w-full h-16 gap-0 sm:gap-3">
<div className="flex flex-row 3xl:flex-1 ">
<button
type="button"
aria-label="Menu"
onClick={() => setIsOpen(!isOpen)}
onClick={() => setIsMenuOpen(!isMenuOpen)}
className={cn(
'active:scale-95 transition-transform flex lg:hidden w-12 h-12 rounded-full items-center justify-center hover:bg-primary/5 hover:dark:bg-primary-dark/5 outline-link',
{
'text-link dark:text-link-dark': isOpen,
'text-link dark:text-link-dark': isMenuOpen,
}
)}>
{isOpen ? <IconClose /> : <IconHamburger />}
{isMenuOpen ? <IconClose /> : <IconHamburger />}
</button>
<div className="flex 3xl:flex-1 align-center">
<NextLink
Expand Down Expand Up @@ -343,14 +343,14 @@ export default function TopNav({
</div>
</nav>

{isOpen && (
{isMenuOpen && (
<div
ref={scrollParentRef}
className="overflow-y-scroll isolate no-bg-scrollbar lg:w-[342px] grow bg-wash dark:bg-wash-dark">
<aside
className={cn(
`lg:grow lg:flex flex-col w-full pb-8 lg:pb-0 lg:max-w-custom-xs z-50`,
isOpen ? 'block z-40' : 'hidden lg:block'
isMenuOpen ? 'block z-40' : 'hidden lg:block'
)}>
<nav
role="navigation"
Expand Down Expand Up @@ -383,10 +383,10 @@ export default function TopNav({
<SidebarRouteTree
// Don't share state between the desktop and mobile versions.
// This avoids unnecessary animations and visual flicker.
key={isOpen ? 'mobile-overlay' : 'desktop-or-hidden'}
key={isMenuOpen ? 'mobile-overlay' : 'desktop-or-hidden'}
routeTree={routeTree}
breadcrumbs={breadcrumbs}
isForceExpanded={isOpen}
isForceExpanded={isMenuOpen}
/>
</Suspense>
<div className="h-16" />
Expand Down
1 change: 0 additions & 1 deletion src/components/Search.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ export function Search({
createPortal(
<DocSearchModal
{...options}
initialScrollY={window.scrollY}
Copy link
Member

Choose a reason for hiding this comment

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

Awesome stuff.

is this the actual fix? Can you explain what the problem was and how this fixes it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rickhanlonii 👋

Thanks so much! Yep, line 113 is the actual fix. I didn't take a look into the internals of the library, but it looks like the bug might actually be in the Autocomplete library. It looks like their DocSearchModal component has a bug in the modal design, which is causing the screen to freeze once the device width is less than about 755px.

width_at_755px_vs_741px.mp4

searchParameters={searchParameters}
onClose={onClose}
navigator={{
Expand Down
Loading