Optimize some parts of Notebook editor (metabase#39441)

* canRun is expensive, reduce number of calls * Drop unused variables * do not calculate TimeSeriesChrome in notebook mode * Do not re-render ColumnNotebookCellItem unnecessary * Do not re-render notebook steps unnecessary * Revert "Do not re-render ColumnNotebookCellItem unnecessary" This reverts commit 09cca64. * Do not render main in notebook mode
ttanay · Mar 1, 2024 · bfd6895 · bfd6895
1 parent afc28b9
commit bfd6895
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 20 deletions.
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.tsx b/frontend/src/metabase/query_builder/components/notebook/NotebookSteps/NotebookSteps.tsx
@@ -63,12 +63,17 @@ function NotebookSteps({
     setLastOpenedStep(id);
   }, []);
 
-  const handleStepClose = useCallback((id: INotebookStep["id"]) => {
-    setOpenSteps(openSteps => ({ ...openSteps, [id]: false }));
-    setLastOpenedStep(lastOpenedStep =>
-      lastOpenedStep === id ? null : lastOpenedStep,
-    );
-  }, []);
+  const handleStepClose = useCallback(
+    (id: INotebookStep["id"]) => {
+      if (openSteps[id]) {
+        setOpenSteps(openSteps => ({ ...openSteps, [id]: false }));
+      }
+      setLastOpenedStep(lastOpenedStep =>
+        lastOpenedStep === id ? null : lastOpenedStep,
+      );
+    },
+    [openSteps],
+  );
 
   const handleQueryChange = useCallback(
     async (query: Query, step: INotebookStep) => {

diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/JoinStep.tsx
@@ -282,7 +282,7 @@ function JoinCondition({
     setOperator,
     setLHSColumn,
     setRHSColumn,
-  } = useJoinCondition(query, stageIndex, table, join, condition);
+  } = useJoinCondition(query, stageIndex, condition);
 
   const getLhsColumnGroup = () => {
     const lhsColumns = Lib.joinConditionLHSColumns(

diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/use-join-condition.ts b/frontend/src/metabase/query_builder/components/notebook/steps/JoinStep/use-join-condition.ts
@@ -6,8 +6,6 @@ import * as Lib from "metabase-lib";
 export function useJoinCondition(
   query: Lib.Query,
   stageIndex: number,
-  table: Lib.Joinable,
-  join?: Lib.Join,
   condition?: Lib.JoinCondition,
 ) {
   const previousCondition = usePrevious(condition);

diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx
@@ -265,8 +265,19 @@ class View extends Component {
   };
 
   renderMain = ({ leftSidebar, rightSidebar }) => {
-    const { question, mode, parameters, isLiveResizable, setParameterValue } =
-      this.props;
+    const {
+      question,
+      mode,
+      parameters,
+      isLiveResizable,
+      setParameterValue,
+      queryBuilderMode,
+    } = this.props;
+
+    if (queryBuilderMode === "notebook") {
+      // we need to render main only in view mode
+      return;
+    }
 
     const queryMode = mode && mode.queryMode();
     const { isNative } = Lib.queryDisplayInfo(question.query());
@@ -295,7 +306,11 @@ class View extends Component {
             mode={queryMode}
           />
         </StyledDebouncedFrame>
-        <TimeseriesChrome {...this.props} className="flex-no-shrink" />
+        <TimeseriesChrome
+          question={this.props.question}
+          updateQuestion={this.props.updateQuestion}
+          className="flex-no-shrink"
+        />
         <ViewFooter {...this.props} className="flex-no-shrink" />
       </QueryBuilderMain>
     );

diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/ViewHeader.jsx b/frontend/src/metabase/query_builder/components/view/ViewHeader/ViewHeader.jsx
@@ -460,11 +460,12 @@ function ViewTitleHeaderRightSide(props) {
     [isRunning],
   );
 
-  const isSaveDisabled = !question.canRun() || !isEditable;
+  const canRun = question.canRun();
+  const isSaveDisabled = !canRun || !isEditable;
   const disabledSaveTooltip = getDisabledSaveTooltip(
-    question,
     isEditable,
     requiredTemplateTags,
+    canRun,
   );
 
   return (
@@ -555,11 +556,7 @@ function ViewTitleHeaderRightSide(props) {
 
 ViewTitleHeader.propTypes = viewTitleHeaderPropTypes;
 
-function getDisabledSaveTooltip(
-  question,
-  isEditable,
-  requiredTemplateTags = [],
-) {
+function getDisabledSaveTooltip(isEditable, requiredTemplateTags = [], canRun) {
   if (!isEditable) {
     return t`You don't have permission to save this question.`;
   }
@@ -568,7 +565,7 @@ function getDisabledSaveTooltip(
     tag => tag.required && !tag.default,
   );
 
-  if (!question.canRun()) {
+  if (!canRun) {
     return getMissingRequiredTemplateTagsTooltip(missingValueRequiredTTags);
   }