Add config option to control the hamburger menu #6174

sfc-gh-kbregula · 2023-02-23T08:35:20Z

📚 Context

Now we have a new option client.toolbarMode that change the visibility of items in the toolbar, options menu, and settings dialog (top right of the app).
Allowed values:

auto : Show the developer options if the app is accessed through localhost and hide them otherwise.
developer : Show the developer options.
viewer : Hide the developer options.
minimal : Show only options set externally (e.g. through Streamlit Community Cloud) or through st.set_page_config. If there are no options left, hide the menu.

Please describe the project or issue background here

What kind of change does this PR introduce?
- Bugfix
- Feature
- Refactoring
- Other, please describe:

🧠 Description of Changes

Add bullet points summarizing your changes here
- This is a breaking API change
- This is a visible (user-facing) change

Revised:

Insert screenshot of your updated UI/code here

Current:

Minimal mode with custom items

🧪 Testing Done

Screenshots included
Added/Updated unit tests
Added/Updated e2e tests

🌐 References

Does this depend on other work, documents, or tickets?

Issue: Closes #XXXX

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

sfc-gh-tszerszen

LGTM 👍

sfc-gh-mnowotka

Left a few comments.

sfc-gh-mnowotka · 2023-02-23T20:00:35Z

frontend/src/components/core/MainMenu/MainMenu.tsx

+  let showDevelopmentMenu: boolean
+  if (props.toolbarMode == Config.ToolbarMode.DEVELOPER) {
+    showDevelopmentMenu = true
+  } else if (
+    props.toolbarMode == Config.ToolbarMode.VIEWER ||
+    props.toolbarMode == Config.ToolbarMode.MINIMAL
+  ) {
+    showDevelopmentMenu = false
+  } else {
+    showDevelopmentMenu = hostIsOwner || isLocalhost()
+  }
+
+  if (menuItems.length == 0 && !showDevelopmentMenu) {
+    // Don't show an empty menu.
+    return <></>
+  }


every time you use let a panda is crying.

function showDeveloperMenu(){ if (props.toolbarMode == Config.ToolbarMode.DEVELOPER) { return true } if ( props.toolbarMode == Config.ToolbarMode.VIEWER || props.toolbarMode == Config.ToolbarMode.MINIMAL ) { return false; } return hostIsOwner || isLocalhost() }

Thanks for suggestion.
I added two commits to my branch. One creates functions to avoid creating let variable, and the second refactors the code a bit more so that we don't need to calculate the menu items when we know the menu is hidden.

sfc-gh-mnowotka · 2023-02-23T20:00:52Z

frontend/src/components/core/MainMenu/MainMenu.tsx

        <>
          <SubMenu menuItems={menuItems} closeMenu={close} isDevMenu={false} />
-          {(hostIsOwner || isLocalhost()) && (
+          {showDevelopmentMenu && (


showDeveloperMenu()

sfc-gh-mnowotka · 2023-02-23T20:05:56Z

lib/streamlit/runtime/app_session.py

    msg.allow_run_on_save = config.get_option("server.allowRunOnSave")
    msg.hide_top_bar = config.get_option("ui.hideTopBar")
    msg.hide_sidebar_nav = config.get_option("ui.hideSidebarNav")
+    msg.toolbar_mode = _get_toolbar_mode()


why this one is different than all the above?

We have an protobuf enum, not a plain boolean/string here, so we need convert config value (string) to enum type.

sfc-gh-mnowotka · 2023-03-02T09:49:24Z

frontend/src/components/core/MainMenu/MainMenu.tsx

+  let preferredMenuOrder: any[]
+  if (props.toolbarMode == Config.ToolbarMode.MINIMAL) {
+    // If toolbar mode == minimal then show only host menu items if any.
+    preferredMenuOrder = shouldShowHostMenu ? hostMenuItems : []
+    // If the first or last item is a divider, delete it, because
+    // we don't want to start/end the menu with it.
+    // TODO(sfc-gh-kbregula): We should use Array#at when supported by
+    //  browsers/cypress or transpilers.
+    //  See: https://github.com/tc39/proposal-relative-indexing-method
+    if (
+      preferredMenuOrder.length > 0 &&
+      preferredMenuOrder[0] == coreMenuItems.DIVIDER
+    ) {
+      preferredMenuOrder.shift()
+    }
+    if (
+      preferredMenuOrder.length > 0 &&
+      preferredMenuOrder.at(preferredMenuOrder.length - 1) ==
+        coreMenuItems.DIVIDER
+    ) {
+      preferredMenuOrder.pop()
+    }
+  } else {
+    preferredMenuOrder = [
+      coreMenuItems.rerun,
+      coreMenuItems.settings,
+      coreMenuItems.DIVIDER,
+      coreMenuItems.print,
+      coreMenuItems.recordScreencast,
+      coreMenuItems.DIVIDER,
+      coreMenuItems.report,
+      coreMenuItems.community,
+      ...(shouldShowHostMenu ? hostMenuItems : [coreMenuItems.DIVIDER]),
+      coreMenuItems.about,
+    ]
+  }


Again, I'd extract this to a separate function that computed the preferred order as this logic got more complex.

sfc-gh-mnowotka · 2023-03-02T09:51:07Z

frontend/src/components/core/MainMenu/MainMenu.tsx

+  const showDevelopmentMenu = (() => {
+    if (props.toolbarMode == Config.ToolbarMode.DEVELOPER) {
+      return true
+    } else if (


nit: if you return in the upper condition you can replace else if with if.

sfc-gh-mnowotka · 2023-03-02T09:51:56Z

frontend/src/components/core/MainMenu/MainMenu.tsx

+      props.toolbarMode == Config.ToolbarMode.MINIMAL
+    ) {
+      return false
+    } else {


nit: you don't need else clause here, you can just write return hostIsOwner || isLocalhost()

In this project, I don't think we have one style convention, but if we want to enforce one style, we should do it automatically, e.g. using the ESLint rule.
https://eslint.org/docs/latest/rules/no-else-return

PR here: #6205

sfc-gh-mnowotka · 2023-03-02T09:53:34Z

frontend/src/components/core/MainMenu/MainMenu.tsx

  const devMenuItems: any[] = []
-  let devLastMenuItem = null
-  for (const devMenuItem of preferredDevMenuOrder) {
-    if (devMenuItem) {
-      if (devMenuItem !== coreDevMenuItems.DIVIDER) {
-        if (devLastMenuItem === coreDevMenuItems.DIVIDER) {
-          devMenuItems.push({ ...devMenuItem, hasDividerAbove: true })
-        } else {
-          devMenuItems.push(devMenuItem)
+
+  if (showDevelopmentMenu) {
+    const preferredDevMenuOrder: any[] = [
+      coreDevMenuItems.developerOptions,
+      coreDevMenuItems.clearCache,
+      showDeploy && coreDevMenuItems.deployApp,
+    ]
+
+    let devLastMenuItem = null
+
+    for (const devMenuItem of preferredDevMenuOrder) {
+      if (devMenuItem) {
+        if (devMenuItem !== coreDevMenuItems.DIVIDER) {
+          if (devLastMenuItem === coreDevMenuItems.DIVIDER) {
+            devMenuItems.push({ ...devMenuItem, hasDividerAbove: true })
+          } else {
+            devMenuItems.push(devMenuItem)
+          }
        }
+
+        devLastMenuItem = devMenuItem
      }
+    }

-      devLastMenuItem = devMenuItem
+    if (devLastMenuItem != null) {
+      devLastMenuItem.styleProps = lastDevMenuItemStyleProps


Again, this looks big and complex enough to get extracted to a separate function.

frontend/src/App.test.tsx

frontend/src/components/core/MainMenu/MainMenu.test.tsx

sfc-gh-kbregula · 2023-03-23T08:32:20Z

frontend/src/App.tsx

This is a dead code because the variable RERUN_PROMPT_MODAL_DIALOG is always set to False. but I would prefer to deal with this problem in a separate PR, because we have to consider whether we can delete it.

frontend/src/App.test.tsx

lib/streamlit/config.py

Co-authored-by: Snehan Kekre <snehan.minerva@gmail.com>

# By Kamil Breguła # Via GitHub * develop: Add config option to control the hamburger menu (streamlit#6174) Restrict the setting of sensitive options by the CLI flag (streamlit#6376) # Conflicts: # frontend/src/app/App.test.tsx # frontend/src/app/App.tsx # frontend/src/app/components/MainMenu/MainMenu.test.tsx # frontend/src/app/components/MainMenu/MainMenu.tsx

sfc-gh-kbregula force-pushed the hide-hamburger branch 2 times, most recently from 71667a1 to 2fa3ef3 Compare February 23, 2023 15:40

sfc-gh-kbregula marked this pull request as ready for review February 23, 2023 16:53

sfc-gh-kbregula requested review from lukasmasuch, sfc-gh-mnowotka, tconkling and vdonato as code owners February 23, 2023 16:53

sfc-gh-tszerszen added the security-assessment-completed Security assessment has been completed for PR label Feb 23, 2023

sfc-gh-tszerszen approved these changes Feb 23, 2023

View reviewed changes

sfc-gh-mnowotka reviewed Feb 23, 2023

View reviewed changes

sfc-gh-kbregula force-pushed the hide-hamburger branch from 6598568 to 30c3f6a Compare March 1, 2023 14:51

sfc-gh-mnowotka reviewed Mar 2, 2023

View reviewed changes

sfc-gh-kbregula mentioned this pull request Mar 2, 2023

Enable no-else-return rule in ESLint #6205

Merged

9 tasks

sfc-gh-mnowotka approved these changes Mar 2, 2023

View reviewed changes

sfc-gh-kbregula mentioned this pull request Mar 9, 2023

Improve deploy button #6223

Merged

9 tasks

sfc-gh-kbregula force-pushed the hide-hamburger branch from 5226849 to 52f8796 Compare March 9, 2023 21:35

sfc-gh-kbregula force-pushed the hide-hamburger branch 2 times, most recently from 3a8df55 to f8557b8 Compare March 23, 2023 08:25

github-advanced-security bot found potential problems Mar 23, 2023

View reviewed changes

frontend/src/App.test.tsx Fixed Show fixed Hide fixed

frontend/src/components/core/MainMenu/MainMenu.test.tsx Fixed Show fixed Hide fixed

sfc-gh-kbregula commented Mar 23, 2023

View reviewed changes

sfc-gh-kbregula force-pushed the hide-hamburger branch 2 times, most recently from ab2957b to 3516a09 Compare March 29, 2023 09:45

github-advanced-security bot found potential problems Mar 29, 2023

View reviewed changes

frontend/src/App.test.tsx Fixed Show fixed Hide fixed

sfc-gh-kbregula force-pushed the hide-hamburger branch from 479f866 to b575e46 Compare March 29, 2023 18:47

sfc-gh-kbregula commented Mar 29, 2023

View reviewed changes

lib/streamlit/config.py Show resolved Hide resolved

snehankekre self-requested a review March 30, 2023 06:08

snehankekre suggested changes Mar 31, 2023

View reviewed changes

lib/streamlit/config.py Outdated Show resolved Hide resolved

sfc-gh-kbregula requested a review from snehankekre April 4, 2023 12:53

snehankekre approved these changes Apr 4, 2023

View reviewed changes

sfc-gh-kbregula and others added 2 commits April 12, 2023 11:29

Add config option to control the hamburger menu

09550d4

Update lib/streamlit/config.py

5e3837d

Co-authored-by: Snehan Kekre <snehan.minerva@gmail.com>

sfc-gh-kbregula force-pushed the hide-hamburger branch from c9823f5 to 5e3837d Compare April 12, 2023 09:36

fixup! Add config option to control the hamburger menu

07c9faf

sfc-gh-mnowotka merged commit 4289ea8 into develop Apr 14, 2023

sfc-gh-jrieke mentioned this pull request Apr 14, 2023

Ability to hide the hamburger menu #395

Closed

sfc-gh-kmcgrady deleted the hide-hamburger branch October 5, 2023 19:30

Add config option to control the hamburger menu #6174

Add config option to control the hamburger menu #6174

Uh oh!

Conversation

sfc-gh-kbregula commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📚 Context

🧠 Description of Changes

🧪 Testing Done

🌐 References

Uh oh!

sfc-gh-tszerszen left a comment

Choose a reason for hiding this comment

Uh oh!

sfc-gh-mnowotka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-kbregula Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sfc-gh-kbregula commented Feb 23, 2023 •

edited

Loading

sfc-gh-kbregula Feb 27, 2023 •

edited

Loading