feat: move bookings tabs to sidebar navigation (#24605)

* feat: move bookings tabs to sidebar navigation

- Add bookings sub-items (Upcoming, Unconfirmed, Recurring, Past, Cancelled) to sidebar navigation
- Remove HorizontalTabs component from bookings view
- Add redirect from /bookings to /bookings/upcoming for default navigation
- Parent Bookings menu item now automatically navigates to Upcoming sub-item

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* clean up

* add e2e tests

* address feedback

* add horizontal tab back

* useBuildHref

* update preserveQueryParams logic

* put back lint rule

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

apps/web/modules/bookings/views/bookings-view.tsx

@@ -23,7 +23,6 @@ import useMeQuery from "@calcom/trpc/react/hooks/useMeQuery";
 import { Alert } from "@calcom/ui/components/alert";
 import type { HorizontalTabItemProps } from "@calcom/ui/components/navigation";
 import { HorizontalTabs } from "@calcom/ui/components/navigation";
-import type { VerticalTabItemProps } from "@calcom/ui/components/navigation";
 import { WipeMyCalActionButton } from "@calcom/web/components/apps/wipemycalother/wipeMyCalActionButton";
 
 import BookingListItem from "@components/booking/BookingListItem";
@@ -96,8 +95,7 @@ function BookingsContent({ status, permissions }: BookingsProps) {
   const user = useMeQuery().data;
   const searchParams = useSearchParams();
 
-  // Generate dynamic tabs that preserve query parameters
-  const tabs: (VerticalTabItemProps | HorizontalTabItemProps)[] = useMemo(() => {
+  const tabs: HorizontalTabItemProps[] = useMemo(() => {
     const queryString = searchParams?.toString() || "";
 
     const baseTabConfigs = [
@@ -392,7 +390,7 @@ function BookingsContent({ status, permissions }: BookingsProps) {
 
   return (
     <div className="flex flex-col">
-      <div className="flex flex-row flex-wrap justify-between">
+      <div className="flex flex-row flex-wrap justify-between lg:hidden">
         <HorizontalTabs
           tabs={tabs.map((tab) => ({
             ...tab,

apps/web/playwright/booking-filters.e2e.ts

@@ -1,5 +1,6 @@
 import { expect } from "@playwright/test";
 
+import { applySelectFilter } from "./filter-helpers";
 import { test } from "./lib/fixtures";
 
 test.describe.configure({ mode: "parallel" });
@@ -15,10 +16,9 @@ test.describe("Booking Filters", () => {
       teammates: [{ name: teamMateName }],
     });
 
-    const allUsers = await users.get();
+    const allUsers = users.get();
     const memberUser = allUsers.find((user) => user.name === teamMateName);
 
-    // eslint-disable-next-line playwright/no-conditional-in-test
     if (!memberUser) {
       throw new Error("user should exist");
     }
@@ -48,4 +48,184 @@ test.describe("Booking Filters", () => {
     await page.locator('[data-testid="add-filter-button"]').click();
     await expect(page.locator('[data-testid="add-filter-item-userId"]')).toBeVisible();
   });
+
+  test("Query params should be preserved when switching between bookings tabs", async ({ page, users }) => {
+    const owner = await users.create(
+      { name: "Owner User" },
+      {
+        hasTeam: true,
+        isOrg: true,
+        teammates: [{ name: "Team Member 1" }, { name: "Team Member 2" }],
+      }
+    );
+
+    await owner.apiLogin();
+
+    const bookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.goto(`/bookings/upcoming`, { waitUntil: "domcontentloaded" });
+    await bookingsGetResponse;
+
+    await applySelectFilter(page, "userId", "Owner User");
+
+    await expect(page).toHaveURL(/.*userId.*/);
+    const urlWithFilters = page.url();
+    expect(urlWithFilters).toContain("userId");
+
+    const searchParams = new URL(urlWithFilters).searchParams;
+    const activeFilters = searchParams.get("activeFilters");
+    expect(activeFilters).toBeTruthy();
+
+    const pastBookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.getByTestId("past-test").click();
+    await expect(page).toHaveURL(/\/bookings\/past/);
+    await pastBookingsGetResponse;
+
+    const pastUrl = page.url();
+    expect(pastUrl).toContain("/bookings/past");
+    expect(pastUrl).toContain("userId");
+
+    const pastSearchParams = new URL(pastUrl).searchParams;
+    expect(pastSearchParams.get("activeFilters")).toBe(activeFilters);
+
+    const cancelledBookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.getByTestId("cancelled-test").click();
+    await expect(page).toHaveURL(/\/bookings\/cancelled/);
+    await cancelledBookingsGetResponse;
+
+    const cancelledUrl = page.url();
+    expect(cancelledUrl).toContain("/bookings/cancelled");
+    expect(cancelledUrl).toContain("userId");
+
+    const cancelledSearchParams = new URL(cancelledUrl).searchParams;
+    expect(cancelledSearchParams.get("activeFilters")).toBe(activeFilters);
+  });
+
+  test("Query params should be preserved when switching between bookings tabs in calendar view", async ({
+    page,
+    users,
+  }) => {
+    const owner = await users.create(
+      { name: "Owner User" },
+      {
+        hasTeam: true,
+        isOrg: true,
+        teammates: [{ name: "Team Member 1" }, { name: "Team Member 2" }],
+      }
+    );
+
+    await owner.apiLogin();
+
+    const bookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.goto(`/bookings/upcoming?view=calendar`, { waitUntil: "domcontentloaded" });
+    await bookingsGetResponse;
+
+    await applySelectFilter(page, "userId", "Owner User");
+
+    await expect(page).toHaveURL(/.*userId.*/);
+    const urlWithFilters = page.url();
+    expect(urlWithFilters).toContain("userId");
+
+    const searchParams = new URL(urlWithFilters).searchParams;
+    const activeFilters = searchParams.get("activeFilters");
+    expect(activeFilters).toBeTruthy();
+    expect(searchParams.get("view")).toBe("calendar");
+
+    const pastBookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.getByTestId("past-test").click();
+    await expect(page).toHaveURL(/\/bookings\/past/);
+    await pastBookingsGetResponse;
+
+    const pastUrl = page.url();
+    expect(pastUrl).toContain("/bookings/past");
+    expect(pastUrl).toContain("userId");
+
+    const pastSearchParams = new URL(pastUrl).searchParams;
+    expect(pastSearchParams.get("activeFilters")).toBe(activeFilters);
+    expect(pastSearchParams.get("view")).toBe("calendar");
+
+    const cancelledBookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.getByTestId("cancelled-test").click();
+    await expect(page).toHaveURL(/\/bookings\/cancelled/);
+    await cancelledBookingsGetResponse;
+
+    const cancelledUrl = page.url();
+    expect(cancelledUrl).toContain("/bookings/cancelled");
+    expect(cancelledUrl).toContain("userId");
+
+    const cancelledSearchParams = new URL(cancelledUrl).searchParams;
+    expect(cancelledSearchParams.get("activeFilters")).toBe(activeFilters);
+    expect(cancelledSearchParams.get("view")).toBe("calendar");
+  });
+
+  test("Query params should NOT be preserved when navigating from a non-bookings page", async ({
+    page,
+    users,
+  }) => {
+    const owner = await users.create(undefined, {
+      hasTeam: true,
+      isOrg: true,
+    });
+
+    await owner.apiLogin();
+    await page.goto(`/event-types?someParam=value`);
+    await expect(page).toHaveURL(/.*someParam=value.*/);
+
+    const upcomingBookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.getByTestId("bookings-test").click();
+    await page.getByTestId("upcoming-test").click();
+    await expect(page).toHaveURL(/\/bookings\/upcoming/);
+    await upcomingBookingsGetResponse;
+
+    const upcomingUrl = page.url();
+    expect(upcomingUrl).toContain("/bookings/upcoming");
+    expect(upcomingUrl).not.toContain("someParam=value");
+  });
+
+  test("Query params should NOT be preserved when navigating from a bookings page to a non-bookings page", async ({
+    page,
+    users,
+  }) => {
+    const owner = await users.create(
+      { name: "Owner User" },
+      {
+        hasTeam: true,
+        isOrg: true,
+        teammates: [{ name: "Team Member 1" }, { name: "Team Member 2" }],
+      }
+    );
+
+    await owner.apiLogin();
+
+    const bookingsGetResponse = page.waitForResponse((response) =>
+      /\/api\/trpc\/bookings\/get.*/.test(response.url())
+    );
+    await page.goto(`/bookings/upcoming`, { waitUntil: "domcontentloaded" });
+    await bookingsGetResponse;
+
+    await applySelectFilter(page, "userId", "Owner User");
+
+    await expect(page).toHaveURL(/.*userId.*/);
+    const urlWithFilters = page.url();
+    expect(urlWithFilters).toContain("userId");
+
+    await page.getByTestId("event_types_page_title-test").click();
+    await expect(page).toHaveURL(/\/event-types/);
+
+    const eventTypeSearchParams = new URL(page.url()).searchParams;
+    expect(eventTypeSearchParams.get("activeFilters")).toBeFalsy();
+  });
 });

packages/features/shell/navigation/Navigation.tsx

@@ -16,6 +16,14 @@ import { NavigationItem, MobileNavigationItem, MobileNavigationMoreItem } from "
 
 export const MORE_SEPARATOR_NAME = "more";
 
+const preserveBookingsQueryParams = ({
+  prevPathname,
+  nextPathname,
+}: {
+  prevPathname: string | null;
+  nextPathname: string;
+}) => Boolean(prevPathname?.startsWith("/bookings/")) && nextPathname.startsWith("/bookings/");
+
 const getNavigationItems = (orgBranding: OrganizationBranding): NavigationItemType[] => [
   {
     name: "event_types_page_title",
@@ -28,6 +36,38 @@ const getNavigationItems = (orgBranding: OrganizationBranding): NavigationItemTy
     icon: "calendar",
     badge: <UnconfirmedBookingBadge />,
     isCurrent: ({ pathname }) => pathname?.startsWith("/bookings") ?? false,
+    child: [
+      {
+        name: "upcoming",
+        href: "/bookings/upcoming",
+        preserveQueryParams: preserveBookingsQueryParams,
+        isCurrent: ({ pathname }) => pathname === "/bookings/upcoming",
+      },
+      {
+        name: "unconfirmed",
+        href: "/bookings/unconfirmed",
+        preserveQueryParams: preserveBookingsQueryParams,
+        isCurrent: ({ pathname }) => pathname === "/bookings/unconfirmed",
+      },
+      {
+        name: "recurring",
+        href: "/bookings/recurring",
+        preserveQueryParams: preserveBookingsQueryParams,
+        isCurrent: ({ pathname }) => pathname === "/bookings/recurring",
+      },
+      {
+        name: "past",
+        href: "/bookings/past",
+        preserveQueryParams: preserveBookingsQueryParams,
+        isCurrent: ({ pathname }) => pathname === "/bookings/past",
+      },
+      {
+        name: "cancelled",
+        href: "/bookings/cancelled",
+        preserveQueryParams: preserveBookingsQueryParams,
+        isCurrent: ({ pathname }) => pathname === "/bookings/cancelled",
+      },
+    ],
   },
   {
     name: "availability",

packages/features/shell/navigation/NavigationItem.tsx

@@ -1,9 +1,10 @@
 import Link from "next/link";
-import { usePathname } from "next/navigation";
-import React, { Fragment, useState, useEffect } from "react";
+import { usePathname, useSearchParams } from "next/navigation";
+import React, { Fragment, useState, useEffect, useRef } from "react";
 
 import { useLocale } from "@calcom/lib/hooks/useLocale";
 import useMediaQuery from "@calcom/lib/hooks/useMediaQuery";
+import { sessionStorage } from "@calcom/lib/webstorage";
 import classNames from "@calcom/ui/classNames";
 import { Badge } from "@calcom/ui/components/badge";
 import { Icon } from "@calcom/ui/components/icon";
@@ -17,26 +18,43 @@ const usePersistedExpansionState = (itemName: string) => {
   const [isExpanded, setIsExpanded] = useState(false);
 
   useEffect(() => {
-    try {
-      // eslint-disable-next-line @calcom/eslint/avoid-web-storage
-      const stored = sessionStorage.getItem(`nav-expansion-${itemName}`);
-      if (stored !== null) {
-        setIsExpanded(JSON.parse(stored));
-      }
-    } catch (_error) {}
+    const stored = sessionStorage.getItem(`nav-expansion-${itemName}`);
+    if (stored !== null) {
+      setIsExpanded(JSON.parse(stored));
+    }
   }, [itemName]);
 
   const setPersistedExpansion = (expanded: boolean) => {
     setIsExpanded(expanded);
-    try {
-      // eslint-disable-next-line @calcom/eslint/avoid-web-storage
-      sessionStorage.setItem(`nav-expansion-${itemName}`, JSON.stringify(expanded));
-    } catch (_error) {}
+    sessionStorage.setItem(`nav-expansion-${itemName}`, JSON.stringify(expanded));
   };
 
   return [isExpanded, setPersistedExpansion] as const;
 };
 
+const useBuildHref = () => {
+  const pathname = usePathname();
+  const searchParams = useSearchParams();
+  const prevPathnameRef = useRef<string | null>(null);
+
+  useEffect(() => {
+    prevPathnameRef.current = pathname;
+  }, [pathname]);
+
+  const buildHref = (childItem: NavigationItemType) => {
+    if (
+      childItem.preserveQueryParams &&
+      childItem.preserveQueryParams({ prevPathname: prevPathnameRef.current, nextPathname: childItem.href })
+    ) {
+      const params = searchParams.toString();
+      return params ? `${childItem.href}?${params}` : childItem.href;
+    }
+    return childItem.href;
+  };
+
+  return buildHref;
+};
+
 export type NavigationItemType = {
   name: string;
   href: string;
@@ -59,6 +77,7 @@ export type NavigationItemType = {
     isChild?: boolean;
     pathname: string | null;
   }) => boolean;
+  preserveQueryParams?: (context: { prevPathname: string | null; nextPathname: string }) => boolean;
 };
 
 const defaultIsCurrent: NavigationItemType["isCurrent"] = ({ isChild, item, pathname }) => {
@@ -77,6 +96,7 @@ export const NavigationItem: React.FC<{
   const current = isCurrent({ isChild: !!isChild, item, pathname });
   const shouldDisplayNavigationItem = useShouldDisplayNavigationItem(props.item);
   const [isExpanded, setIsExpanded] = usePersistedExpansionState(item.name);
+  const buildHref = useBuildHref();
 
   const isTablet = useMediaQuery("(max-width: 1024px)");
   const [isTooltipOpen, setIsTooltipOpen] = useState(false);
@@ -111,7 +131,7 @@ export const NavigationItem: React.FC<{
                     return (
                       <Link
                         key={childItem.name}
-                        href={childItem.href}
+                        href={buildHref(childItem)}
                         aria-current={childIsCurrent ? "page" : undefined}
                         onClick={() => setIsTooltipOpen(false)}
                         className={classNames(
@@ -161,7 +181,9 @@ export const NavigationItem: React.FC<{
               />
             )}
             {isLocaleReady ? (
-              <span className="hidden w-full justify-between truncate text-ellipsis lg:flex">
+              <span
+                className="hidden w-full justify-between truncate text-ellipsis lg:flex"
+                data-testid={`${item.name}-test`}>
                 {t(item.name)}
                 {item.badge && item.badge}
               </span>
@@ -177,7 +199,7 @@ export const NavigationItem: React.FC<{
         <Tooltip side="right" content={t(item.name)} className="lg:hidden">
           <Link
             data-test-id={item.name}
-            href={item.href}
+            href={isChild ? buildHref(item) : item.href}
             aria-label={t(item.name)}
             target={item.target}
             className={classNames(
@@ -272,6 +294,7 @@ export const MobileNavigationMoreItem: React.FC<{
   const { t, isLocaleReady } = useLocale();
   const shouldDisplayNavigationItem = useShouldDisplayNavigationItem(props.item);
   const [isExpanded, setIsExpanded] = usePersistedExpansionState(item.name);
+  const buildHref = useBuildHref();
 
   if (!shouldDisplayNavigationItem) return null;
 
@@ -301,7 +324,7 @@ export const MobileNavigationMoreItem: React.FC<{
               {item.child.map((childItem) => (
                 <li key={childItem.name} className="border-subtle border-t">
                   <Link
-                    href={childItem.href}
+                    href={buildHref(childItem)}
                     className="hover:bg-muted flex items-center p-4 pl-12 transition">
                     <span className="text-default font-medium">
                       {isLocaleReady ? t(childItem.name) : <SkeletonText />}

packages/lib/webstorage.ts

@@ -1,5 +1,5 @@
 /**
- * Provides a wrapper around localStorage(and sessionStorage(TODO when needed)) to avoid errors in case of restricted storage access.
+ * Provides a wrapper around localStorage to avoid errors in case of restricted storage access.
  *
  * TODO: In case of an embed if localStorage is not available(third party), use localStorage of parent(first party) that contains the iframe.
  */
@@ -8,7 +8,7 @@ export const localStorage = {
     try {
       // eslint-disable-next-line @calcom/eslint/avoid-web-storage
       return window.localStorage.getItem(key);
-    } catch (e) {
+    } catch {
       // In case storage is restricted. Possible reasons
       // 1. Third Party Context in Chrome Incognito mode.
       return null;
@@ -18,7 +18,7 @@ export const localStorage = {
     try {
       // eslint-disable-next-line @calcom/eslint/avoid-web-storage
       window.localStorage.setItem(key, value);
-    } catch (e) {
+    } catch {
       // In case storage is restricted. Possible reasons
       // 1. Third Party Context in Chrome Incognito mode.
       // 2. Storage limit reached
@@ -29,7 +29,44 @@ export const localStorage = {
     try {
       // eslint-disable-next-line @calcom/eslint/avoid-web-storage
       window.localStorage.removeItem(key);
-    } catch (e) {
+    } catch {
+      return;
+    }
+  },
+};
+
+/**
+ * Provides a wrapper around localStorage to avoid errors in case of restricted storage access.
+ *
+ * TODO: In case of an embed if localStorage is not available(third party), use localStorage of parent(first party) that contains the iframe.
+ */
+export const sessionStorage = {
+  getItem(key: string) {
+    try {
+      // eslint-disable-next-line @calcom/eslint/avoid-web-storage
+      return window.sessionStorage.getItem(key);
+    } catch {
+      // In case storage is restricted. Possible reasons
+      // 1. Third Party Context in Chrome Incognito mode.
+      return null;
+    }
+  },
+  setItem(key: string, value: string) {
+    try {
+      // eslint-disable-next-line @calcom/eslint/avoid-web-storage
+      window.sessionStorage.setItem(key, value);
+    } catch {
+      // In case storage is restricted. Possible reasons
+      // 1. Third Party Context in Chrome Incognito mode.
+      // 2. Storage limit reached
+      return;
+    }
+  },
+  removeItem: (key: string) => {
+    try {
+      // eslint-disable-next-line @calcom/eslint/avoid-web-storage
+      window.sessionStorage.removeItem(key);
+    } catch {
       return;
     }
   },