Merge branch 'develop' into tim/FormProto

* develop: Feature/sudoless metrics (streamlit#2605) Have st.number_input warn user of mismatched type and format string (streamlit#2604) Remove extra period from sentence (streamlit#2601) Revert "Deploy button front (streamlit#2552)" (streamlit#2599) Fix slider UX (streamlit#2596)
tconkling · Jan 19, 2021 · 2a54c20 · 2a54c20
2 parents 6984abe + 1cff62a
commit 2a54c20
Show file tree

Hide file tree

Showing 47 changed files with 206 additions and 853 deletions.
diff --git a/.gitignore b/.gitignore
@@ -27,7 +27,6 @@ cffi_bin
 
 # Pyenv Stuff
 .python-version
-venv
 
 # Autogenerated Protobufs
 lib/streamlit/proto/*_pb2.py

diff --git a/docs/deploy_streamlit_app.md b/docs/deploy_streamlit_app.md
@@ -141,8 +141,7 @@ Here are some limitations and known issues that we're actively working to resolv
 - **If you're having trouble logging in,** check your Streamlit sharing invitation email and make sure you signed up using your Primary Github email, which you can find [here](https://github.com/settings/emails).
 - When you print something to the terminal, you may need to do a `sys.stdout.flush()` before it shows up.
 - Apps execute in a Linux environment running Debian Buster (slim) with Python 3.7. There is no way to change these, and we may upgrade the environment at any point. If we do upgrade it, we will _usually_ not touch existing apps, so they'll continue to work as expected. But if there's a critical fix in the update, we _may_ force-upgrade all apps.
-- Matplotlib [doesn't work well with threads.](https://matplotlib.org/3.3.2/faq/howto_faq.html#working-with-threads)
-  . So if you're using Matplotlib you should wrap your code with locks as shown in the snippet below. This Matplotlib bug is more prominent when you share your app apps since you're more likely to get more concurrent users then.
+- Matplotlib [doesn't work well with threads](https://matplotlib.org/3.3.2/faq/howto_faq.html#working-with-threads). So if you're using Matplotlib you should wrap your code with locks as shown in the snippet below. This Matplotlib bug is more prominent when you share your app apps since you're more likely to get more concurrent users then.
 
   ```python
   from matplotlib.backends.backend_agg import RendererAgg

diff --git a/e2e/specs/st_main_menu.spec.js b/e2e/specs/st_main_menu.spec.js
@@ -33,7 +33,7 @@ describe("main menu", () => {
     cy.get('[data-testid="main-menu-popover"]').invoke(
       "attr",
       "style",
-      "transform: translate3d(20px, 20px, 0px);"
+      "transform: translate3d(20px, 20px, 0px)"
     );
     cy.get('[data-testid="main-menu-list"]').matchImageSnapshot("main_menu");
 

diff --git a/e2e/specs/st_slider.spec.js b/e2e/specs/st_slider.spec.js
@@ -20,6 +20,12 @@ describe("st.slider", () => {
     cy.visit("http://localhost:3000/");
   });
 
+  it("looks right", () => {
+    cy.get(".stSlider")
+      .first()
+      .matchImageSnapshot("slider");
+  });
+
   it("shows labels", () => {
     cy.get(".stSlider label").should("have.text", "Label 1" + "Label 2");
   });

diff --git a/frontend/cypress/snapshots/linux/2x/st_main_menu.spec.ts/main_menu.snap.png b/frontend/cypress/snapshots/linux/2x/st_main_menu.spec.ts/main_menu.snap.png
diff --git a/frontend/cypress/snapshots/linux/2x/st_slider.spec.js/slider.snap.png b/frontend/cypress/snapshots/linux/2x/st_slider.spec.js/slider.snap.png
diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx
@@ -50,14 +50,13 @@ import {
   ForwardMsgMetadata,
   Initialize,
   NewReport,
+  IDeployParams,
   PageConfig,
   PageInfo,
   SessionEvent,
   WidgetStates,
   SessionState,
   Config,
-  IGitInfo,
-  GitInfo,
 } from "autogen/proto"
 import { without, concat } from "lodash"
 
@@ -104,7 +103,7 @@ interface State {
   initialSidebarState: PageConfig.SidebarState
   allowRunOnSave: boolean
   reportFinishedHandlers: (() => void)[]
-  gitInfo?: IGitInfo | null
+  deployParams?: IDeployParams | null
   pendingFormIds: Set<string>
 }
 
@@ -163,7 +162,7 @@ export class App extends PureComponent<Props, State> {
       initialSidebarState: PageConfig.SidebarState.AUTO,
       allowRunOnSave: true,
       reportFinishedHandlers: [],
-      gitInfo: null,
+      deployParams: null,
       pendingFormIds: new Set<string>(),
     }
 
@@ -248,21 +247,6 @@ export class App extends PureComponent<Props, State> {
     this.openDialog(newDialog)
   }
 
-  showDeployError = (
-    title: string,
-    errorNode: ReactNode,
-    onContinue?: () => void
-  ): void => {
-    this.openDialog({
-      type: DialogType.DEPLOY_ERROR,
-      title,
-      msg: errorNode,
-      onContinue,
-      onClose: () => {},
-      onTryAgain: this.sendLoadGitInfoBackMsg,
-    })
-  }
-
   /**
    * Checks if the code version from the backend is different than the frontend
    */
@@ -307,12 +291,6 @@ export class App extends PureComponent<Props, State> {
     }
   }
 
-  handleGitInfoChanged = (gitInfo: IGitInfo): void => {
-    this.setState({
-      gitInfo,
-    })
-  }
-
   /**
    * Callback when we get a message from the server.
    */
@@ -344,8 +322,6 @@ export class App extends PureComponent<Props, State> {
           this.handlePageConfigChanged(pageConfig),
         pageInfoChanged: (pageInfo: PageInfo) =>
           this.handlePageInfoChanged(pageInfo),
-        gitInfoChanged: (gitInfo: GitInfo) =>
-          this.handleGitInfoChanged(gitInfo),
         reportFinished: (status: ForwardMsg.ReportFinishedStatus) =>
           this.handleReportFinished(status),
         uploadReportProgress: (progress: number) =>
@@ -530,7 +506,12 @@ export class App extends PureComponent<Props, State> {
     }
 
     const { reportHash } = this.state
-    const { reportId, name: reportName, scriptPath } = newReportProto
+    const {
+      reportId,
+      name: reportName,
+      scriptPath,
+      deployParams,
+    } = newReportProto
 
     const newReportHash = hashString(
       SessionInfo.current.installationId + scriptPath
@@ -548,9 +529,10 @@ export class App extends PureComponent<Props, State> {
     if (reportHash === newReportHash) {
       this.setState({
         reportId,
+        deployParams,
       })
     } else {
-      this.clearAppState(newReportHash, reportId, reportName)
+      this.clearAppState(newReportHash, reportId, reportName, deployParams)
     }
   }
 
@@ -630,13 +612,15 @@ export class App extends PureComponent<Props, State> {
   clearAppState(
     reportHash: string,
     reportId: string,
-    reportName: string
+    reportName: string,
+    deployParams?: IDeployParams | null
   ): void {
     this.setState(
       {
         reportId,
         reportName,
         reportHash,
+        deployParams,
         elements: ReportRoot.empty(),
       },
       () => {
@@ -796,19 +780,6 @@ export class App extends PureComponent<Props, State> {
     this.widgetMgr.sendUpdateWidgetsMessage()
   }
 
-  sendLoadGitInfoBackMsg = (): void => {
-    if (!this.isServerConnected()) {
-      logError("Cannot load git information when disconnected from server.")
-      return
-    }
-
-    this.sendBackMsg(
-      new BackMsg({
-        loadGitInfo: true,
-      })
-    )
-  }
-
   sendRerunBackMsg = (widgetStates?: WidgetStates | undefined): void => {
     const { queryParams } = this.props.s4aCommunication.currentState
 
@@ -958,6 +929,7 @@ export class App extends PureComponent<Props, State> {
     const {
       allowRunOnSave,
       connectionState,
+      deployParams,
       dialog,
       elements,
       initialSidebarState,
@@ -967,7 +939,6 @@ export class App extends PureComponent<Props, State> {
       reportRunState,
       sharingEnabled,
       userSettings,
-      gitInfo,
       pendingFormIds,
     } = this.state
     const outerDivClass = classNames("stApp", {
@@ -1029,13 +1000,7 @@ export class App extends PureComponent<Props, State> {
                 screenCastState={this.props.screenCast.currentState}
                 s4aMenuItems={this.props.s4aCommunication.currentState.items}
                 sendS4AMessage={this.props.s4aCommunication.sendMessage}
-                gitInfo={gitInfo}
-                showDeployError={this.showDeployError}
-                closeDialog={this.closeDialog}
-                isDeployErrorModalOpen={
-                  this.state.dialog?.type === DialogType.DEPLOY_ERROR
-                }
-                loadGitInfo={this.sendLoadGitInfoBackMsg}
+                deployParams={deployParams}
               />
             </Header>
 

diff --git a/frontend/src/components/core/MainMenu/MainMenu.test.tsx b/frontend/src/components/core/MainMenu/MainMenu.test.tsx
@@ -18,23 +18,9 @@
 import React from "react"
 import { shallow } from "lib/test_util"
 import { IMenuItem } from "hocs/withS4ACommunication/types"
-import { Args as SessionInfoArgs, SessionInfo } from "lib/SessionInfo"
-
-import { GitInfo, IGitInfo } from "autogen/proto"
-import { IDeployErrorDialog } from "components/core/StreamlitDialog/DeployErrorDialogs/types"
-import {
-  DetachedHead,
-  ModuleIsNotAdded,
-  NoRepositoryDetected,
-  RepoIsAhead,
-  UncommittedChanges,
-  UntrackedFiles,
-} from "components/core/StreamlitDialog/DeployErrorDialogs"
 
 import MainMenu, { Props } from "./MainMenu"
 
-const { GitStates } = GitInfo
-
 const getProps = (extend?: Partial<Props>): Props => ({
   aboutCallback: jest.fn(),
   clearCacheCallback: jest.fn(),
@@ -47,29 +33,10 @@ const getProps = (extend?: Partial<Props>): Props => ({
   settingsCallback: jest.fn(),
   shareCallback: jest.fn(),
   sharingEnabled: false,
-  isDeployErrorModalOpen: false,
-  showDeployError: jest.fn(),
-  loadGitInfo: jest.fn(),
-  closeDialog: jest.fn(),
   ...extend,
 })
 
 describe("App", () => {
-  beforeAll(() => {
-    SessionInfo.current = new SessionInfo({
-      sessionId: "sessionId",
-      streamlitVersion: "sv",
-      pythonVersion: "pv",
-      installationId: "iid",
-      installationIdV1: "iid1",
-      installationIdV2: "iid2",
-      authorEmail: "ae",
-      maxCachedMessageAge: 2,
-      commandLine: "command line",
-      userMapboxToken: "mpt",
-    } as SessionInfoArgs)
-  })
-
   it("renders without crashing", () => {
     const props = getProps()
     const wrapper = shallow(<MainMenu {...props} />)
@@ -160,7 +127,7 @@ describe("App", () => {
   })
 
   it("should render deploy app menu item", () => {
-    const props = getProps({ gitInfo: {} })
+    const props = getProps({ deployParams: {} })
     const wrapper = shallow(<MainMenu {...props} />)
     const popoverContent = wrapper.find("StatefulPopover").prop("content")
     // @ts-ignore
@@ -185,121 +152,4 @@ describe("App", () => {
       "About",
     ])
   })
-
-  describe("Onclick deploy button", () => {
-    function testDeployErrorModal(
-      gitInfo: Partial<IGitInfo>,
-      dialogComponent: (module: string) => IDeployErrorDialog
-    ): void {
-      const props = getProps({
-        gitInfo,
-      })
-      const wrapper = shallow(<MainMenu {...props} />)
-      const popoverContent = wrapper.find("StatefulPopover").prop("content")
-      // @ts-ignore
-      const menuWrapper = shallow(popoverContent(() => {})).dive()
-
-      const items: any = menuWrapper.prop("items")
-
-      const deployOption = items.find(
-        // @ts-ignore
-        ({ label }) => label === "Deploy this app"
-      )
-
-      deployOption.onClick()
-
-      // @ts-ignore
-      const dialog = dialogComponent(props.gitInfo.module)
-
-      expect(props.showDeployError.mock.calls[0][0]).toStrictEqual(
-        dialog.title
-      )
-      expect(props.showDeployError.mock.calls[0][1]).toStrictEqual(dialog.body)
-    }
-
-    it("no repo or remote", () => {
-      testDeployErrorModal(
-        {
-          state: GitStates.DEFAULT,
-        },
-        NoRepositoryDetected
-      )
-    })
-
-    it("empty repo", () => {
-      testDeployErrorModal(
-        {
-          repository: "",
-          branch: "",
-          module: "",
-          state: GitStates.DEFAULT,
-        },
-        NoRepositoryDetected
-      )
-    })
-
-    it("repo is detached", () => {
-      testDeployErrorModal(
-        {
-          repository: "repo",
-          branch: "branch",
-          module: "module",
-          state: GitStates.HEAD_DETACHED,
-        },
-        DetachedHead
-      )
-    })
-
-    it("script was not added to the repo", () => {
-      testDeployErrorModal(
-        {
-          repository: "repo",
-          branch: "branch",
-          module: "module.py",
-          isHeadDetached: false,
-          untrackedFiles: ["module.py"],
-        },
-        ModuleIsNotAdded
-      )
-    })
-
-    it("uncommitted changes", () => {
-      testDeployErrorModal(
-        {
-          repository: "repo",
-          branch: "branch",
-          module: "module.py",
-          isHeadDetached: false,
-          uncommittedFiles: ["module.py"],
-          untrackedFiles: [],
-        },
-        UncommittedChanges
-      )
-    })
-
-    it("changes not pushed to Github", () => {
-      const deployParams: IGitInfo = {
-        repository: "repo",
-        branch: "branch",
-        module: "module.py",
-        uncommittedFiles: [],
-        untrackedFiles: [],
-        state: GitStates.AHEAD_OF_REMOTE,
-      }
-      testDeployErrorModal(deployParams, RepoIsAhead)
-    })
-
-    it("untracked files", () => {
-      testDeployErrorModal(
-        {
-          repository: "repo",
-          branch: "branch",
-          module: "module.py",
-          isHeadDetached: false,
-          untrackedFiles: ["another-file.py"],
-        },
-        UntrackedFiles
-      )
-    })
-  })
 })