Fix bugs in the <Tabs /> component. (#400)

mlcommons · Nov 28, 2023 · 77fd5a7 · 77fd5a7
1 parent 1143bcb
commit 77fd5a7
Show file tree

Hide file tree

Showing 17 changed files with 112 additions and 74 deletions.
diff --git a/editor/app.py b/editor/app.py
@@ -56,6 +56,8 @@ def _back_to_menu():
 def _logout():
     """Logs the user out."""
     st.cache_data.clear()
+    get_cached_user.clear()
+    st.session_state[User] = None
     _back_to_menu()
 
 
@@ -70,7 +72,9 @@ def _logout():
     col3.button("Menu", on_click=_back_to_menu)
 
 
-if st.session_state.get(CurrentProject):
+should_display_editor = bool(st.session_state.get(CurrentProject))
+
+if should_display_editor:
     render_editor()
 else:
     render_splash()
diff --git a/editor/components/tabs/__init__.py b/editor/components/tabs/__init__.py
@@ -2,6 +2,8 @@
 
 import streamlit.components.v1 as components
 
+from core.constants import OVERVIEW
+
 # Create a _RELEASE constant. We'll set this to False while we're developing
 # the component, and True when we're ready to package and distribute it.
 _RELEASE = True
@@ -17,12 +19,12 @@
     _component_func = components.declare_component("tabs_component", path=build_dir)
 
 
-def render_tabs(tabs: list[str], selected_tab: int, key=None):
+def render_tabs(tabs: list[str], selected_tab: int, json: str | None, key=None):
     """Create a new instance of "tabs_component".
 
     Args:
-        nodes: The nodes to render in the tree. Nodes are dictionaries with keys `name`
-            (unique identifier), `type` and `parent` (referencing another name).
+        tabs: The tabs to render in the component.
+        selected_tab: The selected tab.
         key: An optional key that uniquely identifies this component. If this is
             None, and the component's arguments are changed, the component will
             be re-mounted in the Streamlit frontend and lose its current state.
@@ -33,6 +35,10 @@ def render_tabs(tabs: list[str], selected_tab: int, key=None):
             frontend.)
     """
     component_value = _component_func(
-        tabs=tabs, selected_tab=selected_tab, key=key, default=0
+        tabs=tabs,
+        selected_tab=selected_tab,
+        json=json,
+        key=key,
+        default=OVERVIEW,
     )
     return component_value
diff --git a/editor/components/tabs/frontend/build/asset-manifest.json b/editor/components/tabs/frontend/build/asset-manifest.json
@@ -1,10 +1,10 @@
 {
   "files": {
-    "main.js": "./static/js/main.d73ef39a.js",
+    "main.js": "./static/js/main.716a0ab4.js",
     "index.html": "./index.html",
-    "main.d73ef39a.js.map": "./static/js/main.d73ef39a.js.map"
+    "main.716a0ab4.js.map": "./static/js/main.716a0ab4.js.map"
   },
   "entrypoints": [
-    "static/js/main.d73ef39a.js"
+    "static/js/main.716a0ab4.js"
   ]
 }
diff --git a/editor/components/tabs/frontend/build/index.html b/editor/components/tabs/frontend/build/index.html
@@ -1 +1 @@
-<!doctype html><html lang="en"><head><title>Streamlit Tabs Component</title><meta charset="UTF-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><meta name="description" content="Streamlit Tree Component"/><script defer="defer" src="./static/js/main.d73ef39a.js"></script></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html>
+<!doctype html><html lang="en"><head><title>Streamlit Tabs Component</title><meta charset="UTF-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><meta name="description" content="Streamlit Tree Component"/><script defer="defer" src="./static/js/main.716a0ab4.js"></script></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html>
diff --git a/...frontend/build/static/js/main.d73ef39a.js → ...frontend/build/static/js/main.716a0ab4.js b/...frontend/build/static/js/main.d73ef39a.js → ...frontend/build/static/js/main.716a0ab4.js
diff --git a/...ld/static/js/main.d73ef39a.js.LICENSE.txt → ...ld/static/js/main.716a0ab4.js.LICENSE.txt b/...ld/static/js/main.d73ef39a.js.LICENSE.txt → ...ld/static/js/main.716a0ab4.js.LICENSE.txt
diff --git a/editor/components/tabs/frontend/build/static/js/main.716a0ab4.js.map b/editor/components/tabs/frontend/build/static/js/main.716a0ab4.js.map
diff --git a/editor/components/tabs/frontend/build/static/js/main.d73ef39a.js.map b/editor/components/tabs/frontend/build/static/js/main.d73ef39a.js.map
diff --git a/editor/components/tabs/frontend/src/Tabs.tsx b/editor/components/tabs/frontend/src/Tabs.tsx
@@ -4,6 +4,7 @@ import {
   withStreamlitConnection,
 } from "streamlit-component-lib"
 import React, { ReactNode } from "react"
+import Button from "@mui/material/Button"
 import Tabs from "@mui/material/Tabs"
 import Tab from "@mui/material/Tab"
 import Box from "@mui/material/Box"
@@ -19,9 +20,11 @@ const theme = createTheme({
 function BasicTabs({
   tabs,
   selectedTab,
+  json,
 }: {
   tabs: string[]
   selectedTab: number
+  json?: { name: string; content: string }
 }) {
   const [value, setValue] = React.useState(selectedTab)
   const handleChange = (event: React.SyntheticEvent, newValue: number) => {
@@ -30,29 +33,52 @@ function BasicTabs({
   }
 
   return (
-    <Box sx={{ width: "100%", margin: -1, padding: 0 }}>
-      <Box sx={{ borderBottom: 1, borderColor: "divider" }}>
-        <Tabs
-          value={value}
-          onChange={handleChange}
-          aria-label="navigation-tabs"
-        >
-          {tabs.map((tab) => (
-            <Tab key={`custom-tab-${tab}`} label={tab} />
-          ))}
-        </Tabs>
+    <div
+      style={{
+        display: "flex",
+        flexDirection: "row",
+        justifyContent: "center",
+        alignItems: "center",
+        marginTop: -8,
+      }}
+    >
+      <Box sx={{ width: "100%", margin: -1, padding: 0 }}>
+        <Box sx={{ borderBottom: 1, borderColor: "divider" }}>
+          <Tabs
+            value={value}
+            onChange={handleChange}
+            aria-label="navigation-tabs"
+          >
+            {tabs.map((tab) => (
+              <Tab key={`custom-tab-${tab}`} label={tab} />
+            ))}
+          </Tabs>
+        </Box>
       </Box>
-    </Box>
+      <Button
+        disabled={!json}
+        variant="outlined"
+        href={
+          json
+            ? `data:text/json;charset=utf-8,${encodeURIComponent(json.content)}`
+            : ""
+        }
+        download={json ? json.name : ""}
+      >
+        Export
+      </Button>
+    </div>
   )
 }
 
 class StreamlitTabs extends StreamlitComponentBase<{}> {
   public render = (): ReactNode => {
     const tabs = this.props.args["tabs"]
     const selectedTab = this.props.args["selected_tab"]
+    const json = this.props.args["json"]
     return (
       <ThemeProvider theme={theme}>
-        <BasicTabs tabs={tabs} selectedTab={selectedTab} />
+        <BasicTabs tabs={tabs} selectedTab={selectedTab} json={json} />
       </ThemeProvider>
     )
   }

diff --git a/editor/core/query_params.py b/editor/core/query_params.py
@@ -26,6 +26,9 @@ def _get_query_param(params: dict[str, Any], name: str) -> str | None:
 
 def _set_query_param(param: str, new_value: str) -> str | None:
     params = st.experimental_get_query_params()
+    if params.get(param) == [new_value]:
+        # The value already exists in the query params.
+        return
     new_params = {k: v for k, v in params.items() if k != param}
     new_params[param] = new_value
     st.experimental_set_query_params(**new_params)

diff --git a/editor/core/state.py b/editor/core/state.py
@@ -335,7 +335,11 @@ class OpenTab:
 
 
 def get_tab():
-    return st.session_state.get(OpenTab, 0)
+    tab = st.session_state.get(OpenTab)
+    if tab is None:
+        return 0
+    else:
+        return tab
 
 
 def set_tab(tab: str):

diff --git a/editor/cypress/e2e/displayErrors.cy.js b/editor/cypress/e2e/displayErrors.cy.js
@@ -27,10 +27,7 @@ describe('load existing errored croissant', () => {
     })
     cy.contains('split_enums (2 fields)').click()
     cy.get('input[aria-label="Name:red[*]"][value="split_enums"]').should('be.visible').type('{selectall}{backspace}{enter}')
-    // TODO(marcenacp): Need to first click `RecordSets`, before being able to click `Overview`.
-    cy.enter('[title="components.tabs.tabs_component"]').then(getBody => {
-      getBody().contains('RecordSets').click({force: true})
-    })
+    cy.wait(2000)
     cy.enter('[title="components.tabs.tabs_component"]').then(getBody => {
       getBody().contains('Overview').click({force: true})
     })

diff --git a/editor/cypress/e2e/loadCroissant.cy.js b/editor/cypress/e2e/loadCroissant.cy.js
@@ -50,7 +50,9 @@ describe('Editor loads Croissant without Error', () => {
 
     cy.get('[data-testid="stException"]').should('not.exist')
 
-    cy.get('button').contains('Export').should('exist').should('be.visible').click({force: true})
+    cy.enter('[title="components.tabs.tabs_component"]').then(getBody => {
+      getBody().contains('Export').click()
+    })
     cy.fixture('titanic.json').then((fileContent) => {
       const downloadsFolder = Cypress.config("downloadsFolder");
       cy.readFile(path.join(downloadsFolder, "croissant-titanic.json"))

diff --git a/editor/deploy_to_hf.sh b/editor/deploy_to_hf.sh
@@ -3,12 +3,15 @@ echo "Deleting $HF_REPO..."
 rm -rf ${HF_REPO}
 git clone git@hf.co:spaces/marcenacp/croissant-editor ${HF_REPO}
 echo "Copying files from $PWD to $HF_REPO..."
-rsync -aP --exclude="README.md" --exclude="*node_modules*" --exclude="*__pycache__*" . ${HF_REPO}
+rsync -aP --exclude="README.md" --exclude="*node_modules*" --exclude="cypress/*" --exclude="*__pycache__*" . ${HF_REPO}
 cd ${HF_REPO}
-echo "Now push with: 'cd $HF_REPO && git add && git commit && git push'."
+git add .
+git commit -m "Deploy (see actual commits on https://github.com/mlcommons/croissant)."
+echo "Now push with: 'cd $HF_REPO && git push'."
 echo "Warning: if it fails, you may need to follow https://huggingface.co/docs/hub/security-git-ssh#generating-a-new-ssh-keypair"
 echo "On Hugging Face Spaces, you might have to set the following environment variables:"
 echo "- REDIRECT_URI"
 echo "- OAUTH_STATE"
 echo "- OAUTH_CLIENT_ID"
 echo "- OAUTH_CLIENT_SECRET"
+echo "Visit: https://huggingface.co/spaces/marcenacp/croissant-editor"
diff --git a/editor/utils.py b/editor/utils.py
@@ -18,17 +18,14 @@ def init_state(force=False):
     """Initializes the session state. `force=True` to force re-initializing it."""
 
     timestamp = get_project_timestamp()
-    if timestamp and not force:
-        project = CurrentProject.from_timestamp(timestamp)
-        if (
-            project
-            and CurrentProject not in st.session_state
-            and Metadata not in st.session_state
-        ):
-            st.session_state[CurrentProject] = project
-            st.session_state[Metadata] = open_project(project.path)
-    else:
-        st.session_state[CurrentProject] = None
+    if CurrentProject not in st.session_state or force:
+        if timestamp:
+            project = CurrentProject.from_timestamp(timestamp)
+            if project:
+                st.session_state[CurrentProject] = project
+                st.session_state[Metadata] = open_project(project.path)
+        else:
+            st.session_state[CurrentProject] = None
 
     if Metadata not in st.session_state or force:
         st.session_state[Metadata] = Metadata()

diff --git a/editor/views/record_sets.py b/editor/views/record_sets.py
@@ -92,9 +92,7 @@ def _handle_create_record_set():
     metadata.add_record_set(RecordSet(name="new-record-set", description=""))
 
 
-def _handle_fields_change(
-    record_set_key: int, record_set: RecordSet, params: dict[str, Any]
-):
+def _handle_fields_change(record_set_key: int, record_set: RecordSet):
     expand_record_set(record_set=record_set)
     data_editor_key = _data_editor_key(record_set_key, record_set)
     result = st.session_state[data_editor_key]
@@ -250,8 +248,7 @@ def _render_left_panel():
             )
             st.data_editor(
                 fields,
-                # There is a bug with `st.data_editor` when the df is empty.
-                use_container_width=not fields.empty,
+                use_container_width=True,
                 num_rows="dynamic",
                 key=data_editor_key,
                 column_config={

diff --git a/editor/views/wizard.py b/editor/views/wizard.py
@@ -20,34 +20,33 @@
 from views.record_sets import render_record_sets
 
 
-def render_export_button(col):
+def _export_json() -> str | None:
     metadata: Metadata = st.session_state[Metadata]
     try:
-        col.download_button(
-            "Export",
-            file_name=f"croissant-{metadata.name.lower()}.json",
-            type="primary",
-            data=json.dumps(metadata.to_canonical().to_json()),
-            help="Export the Croissant JSON-LD",
-        )
+        return {
+            "name": f"croissant-{metadata.name.lower()}.json",
+            "content": json.dumps(metadata.to_canonical().to_json()),
+        }
     except mlc.ValidationError as exception:
-        col.download_button("Export", disabled=True, data="", help=str(exception))
+        return None
 
 
 def render_editor():
-    col1, col2 = st.columns([10, 1])
-    render_export_button(col2)
+    export_json = _export_json()
 
-    with col1:
-        selected_tab = get_tab()
-        selected_tab = render_tabs(tabs=TABS, selected_tab=selected_tab, key="tabs")
-        if selected_tab == OVERVIEW or not selected_tab:
-            render_overview()
-        elif selected_tab == METADATA:
-            render_metadata()
-        elif selected_tab == RESOURCES:
-            render_files()
-        elif selected_tab == RECORD_SETS:
-            render_record_sets()
-        save_current_project()
-        set_tab(selected_tab)
+    # Warning: the custom component cannot be nested in a st.columns or it is forced to
+    # re-render even if a `key` is set.
+    selected_tab = get_tab()
+    tab = render_tabs(
+        tabs=TABS, selected_tab=selected_tab, json=export_json, key="tabs"
+    )
+    if tab == OVERVIEW:
+        render_overview()
+    elif tab == METADATA:
+        render_metadata()
+    elif tab == RESOURCES:
+        render_files()
+    elif tab == RECORD_SETS:
+        render_record_sets()
+    save_current_project()
+    set_tab(tab)
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		<!doctype html><html lang="en"><head><title>Streamlit Tabs Component</title><meta charset="UTF-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><meta name="description" content="Streamlit Tree Component"/><script defer="defer" src="./static/js/main.d73ef39a.js"></script></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html>
		<!doctype html><html lang="en"><head><title>Streamlit Tabs Component</title><meta charset="UTF-8"/><meta name="viewport" content="width=device-width,initial-scale=1"/><meta name="theme-color" content="#000000"/><meta name="description" content="Streamlit Tree Component"/><script defer="defer" src="./static/js/main.716a0ab4.js"></script></head><body><noscript>You need to enable JavaScript to run this app.</noscript><div id="root"></div></body></html>