Refactor SidebarNav

e2e_playwright/hello_app_test.py

@@ -35,7 +35,7 @@
     check_page_title(app, "Welcome to Streamlit!")
     expect(app.get_by_test_id("stSidebar")).to_be_visible()
 
     assert_snapshot(app, name="hello_app-home_page")
 
 
 def test_animation_demo_page(app: Page, assert_snapshot: ImageCompareFunction) -> None:
@@ -47,7 +47,7 @@
     # which is a lot more than the default timeout, so we set it to a higher value
     expect(app.get_by_test_id("stButton")).to_contain_text("Re-run", timeout=25000)
 
     assert_snapshot(app, name="hello_app-animation_demo_page")
 
 
 def test_plotting_demo_page(app: Page, assert_snapshot: ImageCompareFunction) -> None:
@@ -63,7 +63,7 @@
         app.get_by_test_id("stArrowVegaLiteChart").locator("canvas")
     ).to_have_attribute("height", "350")
 
     assert_snapshot(app, name="hello_app-plotting_demo_page")
 
 
 def test_mapping_demo_page(app: Page) -> None:
@@ -146,9 +146,10 @@
     app = themed_app
     _load_dataframe_demo_page(app)
     # close sidebar. Must be done before print-mode, because we hide the close button when printing
-    sidebar_element = app.get_by_test_id("stSidebarContent")
-    sidebar_element.get_by_test_id("baseButton-header").click()
-    expect(sidebar_element).not_to_be_visible()
+    sidebar_header = app.get_by_test_id("stSidebarHeader")
+    sidebar_header.hover()
+    sidebar_header.get_by_test_id("baseButton-header").click()
+    expect(app.get_by_test_id("stSidebarContent")).not_to_be_visible()
 
     app.emulate_media(media="print", forced_colors="active")
     _set_portrait_dimensions(app)
@@ -180,9 +181,10 @@
     app = themed_app
     _load_dataframe_demo_page(app)
     # close sidebar. Must be done before print-mode, because we hide the close button when printing
-    sidebar_element = app.get_by_test_id("stSidebarContent")
-    sidebar_element.get_by_test_id("baseButton-header").click()
-    expect(sidebar_element).not_to_be_visible()
+    sidebar_header = app.get_by_test_id("stSidebarHeader")
+    sidebar_header.hover()
+    sidebar_header.get_by_test_id("baseButton-header").click()
+    expect(app.get_by_test_id("stSidebarContent")).not_to_be_visible()
 
     app.emulate_media(media="print", forced_colors="active")
     _set_landscape_dimensions(app)

e2e_playwright/multipage_apps/mpa_basics_test.py

@@ -30,7 +30,7 @@
     themed_app: Page, assert_snapshot: ImageCompareFunction
 ):
     """Test that the sidebar nav is rendered correctly."""
     assert_snapshot(themed_app.get_by_test_id("stSidebarNav"), name="mpa-sidebar_nav")
 
 
 def test_can_switch_between_pages_by_clicking_on_sidebar_links(app: Page):
@@ -108,29 +108,32 @@
     page.goto(f"http://localhost:{app_port}/page_7")
     wait_for_app_loaded(page)
 
-    separator = page.get_by_test_id("stSidebarNavSeparator")
-    svg = separator.locator("svg")
+    page.get_by_test_id("stSidebarNavSeparator")
+    expand_button = page.get_by_test_id("stSidebarNavViewMore")
 
-    expect(svg).to_be_visible()
+    expect(expand_button).to_be_visible()
 
     # Expand the nav
-    svg.click(force=True)
+    expand_button.click(force=True)
     # We apply a quick timeout here so that the UI has some time to
     # adjust for the screenshot after the click
     page.wait_for_timeout(250)
     assert_snapshot(
         page.get_by_test_id("stSidebarNav"), name="mpa-sidebar_nav_expanded"
     )
 
+    collapse_button = page.get_by_test_id("stSidebarNavViewLess")
+    expect(collapse_button).to_be_visible()
+
     # Collapse the nav
-    svg.click(force=True)
+    collapse_button.click(force=True)
     page.wait_for_timeout(250)
     assert_snapshot(
         page.get_by_test_id("stSidebarNav"), name="mpa-sidebar_nav_collapsed"
     )
 
     # Expand the nav again
-    svg.click(force=True)
+    page.get_by_test_id("stSidebarNavViewMore").click(force=True)
     page.wait_for_timeout(250)
     assert_snapshot(
         page.get_by_test_id("stSidebarNav"), name="mpa-sidebar_nav_expanded"

frontend/app/src/components/Sidebar/Sidebar.test.tsx

@@ -26,7 +26,6 @@ import {
 import {
   render,
   mockEndpoints,
-  useIsOverflowing,
   emotionLightTheme,
   PageConfig,
 } from "@streamlit/lib"
@@ -38,10 +37,6 @@ jest.mock("@streamlit/lib/src/util/Hooks", () => ({
   useIsOverflowing: jest.fn(),
 }))
 
-const mockUseIsOverflowing = useIsOverflowing as jest.MockedFunction<
-  typeof useIsOverflowing
->
-
 function renderSidebar(props: Partial<SidebarProps> = {}): RenderResult {
   return render(
     <Sidebar
@@ -97,7 +92,8 @@ describe("Sidebar Component", () => {
       "true"
     )
 
-    // Click the close sidebar X
+    // Click the close sidebar <
+    fireEvent.mouseOver(screen.getByTestId("stSidebarHeader"))
     fireEvent.click(screen.getByRole("button"))
 
     expect(screen.getByTestId("stSidebar")).toHaveAttribute(
@@ -136,37 +132,38 @@ describe("Sidebar Component", () => {
     expect(screen.queryByTestId("collapsedControl")).not.toBeInTheDocument()
   })
 
-  it("hides scrollbar when hideScrollbar is called", () => {
+  it("shows/hides the collapse arrow when hovering over top of sidebar", () => {
     const appPages = [
       { pageName: "first_page", pageScriptHash: "page_hash" },
       { pageName: "second_page", pageScriptHash: "page_hash2" },
     ]
-    // Need isOverflowing in SidebarNav to be true to test scrollbar behavior
-    mockUseIsOverflowing.mockReturnValueOnce(true)
     renderSidebar({ appPages })
 
-    expect(screen.getByTestId("stSidebarContent")).toHaveStyle(
-      "overflow: overlay"
+    // Hidden when not hovering near the top of sidebar
+    expect(screen.getByTestId("stSidebarCollapseButton")).toHaveStyle(
+      "display: none"
     )
 
-    fireEvent.mouseOver(screen.getByTestId("stSidebarNavItems"))
+    // Hover over the sidebar header
+    fireEvent.mouseOver(screen.getByTestId("stSidebarHeader"))
 
-    expect(screen.getByTestId("stSidebarContent")).toHaveStyle(
-      "overflow: hidden"
+    // Displays the collapse <
+    expect(screen.getByTestId("stSidebarCollapseButton")).toHaveStyle(
+      "display: inline"
     )
   })
 
-  it("has extra top padding if no SidebarNav is displayed", () => {
+  it("has no top padding if no SidebarNav is displayed", () => {
     renderSidebar({
       appPages: [{ pageName: "streamlit_app", pageScriptHash: "page_hash" }],
     })
 
     expect(screen.getByTestId("stSidebarUserContent")).toHaveStyle(
-      "padding-top: 6rem"
+      "padding-top: 0"
     )
   })
 
-  it("has less padding if the SidebarNav is displayed", () => {
+  it("has small padding if the SidebarNav is displayed", () => {
     renderSidebar({
       appPages: [
         { pageName: "streamlit_app", pageScriptHash: "page_hash" },
@@ -175,7 +172,7 @@ describe("Sidebar Component", () => {
     })
 
     expect(screen.getByTestId("stSidebarUserContent")).toHaveStyle(
-      "padding-top: 1rem"
+      "padding-top: 1.5rem"
     )
   })
 
@@ -221,7 +218,7 @@ describe("Sidebar Component", () => {
 
     expect(screen.queryByTestId("stSidebarNav")).not.toBeInTheDocument()
     expect(screen.getByTestId("stSidebarUserContent")).toHaveStyle(
-      "padding-top: 6rem"
+      "padding-top: 0"
     )
   })
 })

frontend/app/src/components/Sidebar/Sidebar.tsx

@@ -15,7 +15,7 @@
  */
 
 import React, { PureComponent, ReactElement, ReactNode } from "react"
-import { ChevronRight, Close } from "@emotion-icons/material-outlined"
+import { ChevronRight, ChevronLeft } from "@emotion-icons/material-outlined"
 import { withTheme } from "@emotion/react"
 import { Resizable } from "re-resizable"
 
@@ -35,11 +35,13 @@ import {
 
 import {
   StyledSidebar,
-  StyledSidebarCloseButton,
   StyledSidebarContent,
   StyledSidebarCollapsedControl,
   StyledSidebarUserContent,
   StyledResizeHandle,
+  StyledSidebarHeaderContainer,
+  StyledCollapseSidebarButton,
+  StyledNoLogoSpacer,
 } from "./styled-components"
 import SidebarNav from "./SidebarNav"
 
@@ -61,8 +63,8 @@ interface State {
   sidebarWidth: string
   lastInnerWidth: number
 
-  // When hovering the nav
-  hideScrollbar: boolean
+  // When hovering sidebar header
+  showSidebarCollapse: boolean
 }
 
 class Sidebar extends PureComponent<SidebarProps, State> {
@@ -91,7 +93,7 @@ class Sidebar extends PureComponent<SidebarProps, State> {
       collapsedSidebar: Sidebar.shouldCollapse(props, this.mediumBreakpointPx),
       sidebarWidth: cachedSidebarWidth || Sidebar.minWidth,
       lastInnerWidth: window ? window.innerWidth : Infinity,
-      hideScrollbar: false,
+      showSidebarCollapse: false,
     }
   }
 
@@ -199,8 +201,12 @@ class Sidebar extends PureComponent<SidebarProps, State> {
     this.setState({ collapsedSidebar: !collapsedSidebar })
   }
 
-  hideScrollbar = (newValue: boolean): void => {
-    this.setState({ hideScrollbar: newValue })
+  onMouseOver = (): void => {
+    this.setState({ showSidebarCollapse: true })
+  }
+
+  onMouseOut = (): void => {
+    this.setState({ showSidebarCollapse: false })
   }
 
   // Additional safeguard for sidebar height sizing
@@ -218,7 +224,7 @@ class Sidebar extends PureComponent<SidebarProps, State> {
   }
 
   public render(): ReactNode {
-    const { collapsedSidebar, sidebarWidth, hideScrollbar } = this.state
+    const { collapsedSidebar, sidebarWidth, showSidebarCollapse } = this.state
     const {
       appPages,
       chevronDownshift,
@@ -283,31 +289,39 @@ class Sidebar extends PureComponent<SidebarProps, State> {
         >
           <StyledSidebarContent
             data-testid="stSidebarContent"
-            hideScrollbar={hideScrollbar}
             ref={this.sidebarRef}
           >
-            <StyledSidebarCloseButton>
-              <BaseButton
-                kind={BaseButtonKind.HEADER_BUTTON}
-                onClick={this.toggleCollapse}
+            <StyledSidebarHeaderContainer
+              onMouseOver={this.onMouseOver}
+              onMouseOut={this.onMouseOut}
+              data-testid="stSidebarHeader"
+            >
+              <StyledNoLogoSpacer data-testid="stLogoSpacer" />
+              <StyledCollapseSidebarButton
+                showSidebarCollapse={showSidebarCollapse}
+                data-testid="stSidebarCollapseButton"
               >
-                <Icon content={Close} size="lg" />
-              </BaseButton>
-            </StyledSidebarCloseButton>
+                <BaseButton
+                  kind={BaseButtonKind.HEADER_BUTTON}
+                  onClick={this.toggleCollapse}
+                >
+                  <Icon content={ChevronLeft} size="xl" />
+                </BaseButton>
+              </StyledCollapseSidebarButton>
+            </StyledSidebarHeaderContainer>
             {!hideSidebarNav && (
               <SidebarNav
                 endpoints={this.props.endpoints}
                 appPages={appPages}
                 collapseSidebar={this.toggleCollapse}
                 currentPageScriptHash={currentPageScriptHash}
                 hasSidebarElements={hasElements}
-                hideParentScrollbar={this.hideScrollbar}
                 onPageChange={onPageChange}
               />
             )}
             <StyledSidebarUserContent
-              data-testid="stSidebarUserContent"
               hasPageNavAbove={hasPageNavAbove}
+              data-testid="stSidebarUserContent"
             >
               {children}
             </StyledSidebarUserContent>