Make filters sticky in Dashboard page (#17406)

frontend/src/metabase/components/TokenField.jsx

@@ -452,6 +452,8 @@ export default class TokenField extends Component {
   }
 
   componentDidUpdate(prevProps: Props, prevState: State) {
+    const input = this.inputRef.current;
+
     if (
       prevState.selectedOptionValue !== this.state.selectedOptionValue &&
       this.scrollElement != null
@@ -461,13 +463,22 @@ export default class TokenField extends Component {
         element.scrollIntoView({ block: "nearest" });
       }
     }
+
     // if we added a value then scroll to the last item (the input)
     if (this.props.value.length > prevProps.value.length) {
-      const input = this.inputRef.current;
       if (input && isObscured(input)) {
         input.scrollIntoView({ block: "nearest" });
       }
     }
+
+    // We focus on the input here, and not on the input itself as a prop
+    // (say by passing prop autoFocus={isFocused})
+    // because certain TokenFields will live in position: fixed containers.
+    // Autofocusing like that would make the page jump in scroll position.
+    // One example: parameter filters in dashboard pages.
+    if (this.state.isFocused) {
+      input.focus({ preventScroll: true });
+    }
   }
 
   render() {
@@ -575,7 +586,6 @@ export default class TokenField extends Component {
             size={10}
             placeholder={placeholder}
             value={inputValue}
-            autoFocus={isFocused}
             onKeyDown={this.onInputKeyDown}
             onChange={this.onInputChange}
             onFocus={this.onInputFocus}

frontend/src/metabase/css/dashboard.css

@@ -95,10 +95,6 @@
   transition: background-color 1s linear, border 1s linear;
 }
 
-.DashboardGrid {
-  margin-top: 1em;
-}
-
 .Dash--editing {
   margin-top: 1.5em;
 }

frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx

@@ -18,6 +18,9 @@ import {
 import DashboardGrid from "../DashboardGrid";
 import ParametersWidget from "./ParametersWidget/ParametersWidget";
 import DashboardEmptyState from "./DashboardEmptyState/DashboardEmptyState";
+import { updateParametersWidgetStickiness } from "./stickyParameters";
+
+const SCROLL_THROTTLE_INTERVAL = 1000 / 24;
 
 // NOTE: move DashboardControls HoC to container
 @DashboardControls
@@ -25,6 +28,7 @@ export default class Dashboard extends Component {
   state = {
     error: null,
     showAddQuestionSidebar: false,
+    isParametersWidgetSticky: false,
   };
 
   static propTypes = {
@@ -74,9 +78,27 @@ export default class Dashboard extends Component {
     isSharing: false,
   };
 
+  constructor(props) {
+    super(props);
+    this.parametersWidgetRef = React.createRef();
+    this.parametersAndCardsContainerRef = React.createRef();
+  }
+
   // NOTE: all of these lifecycle methods should be replaced with DashboardData HoC in container
   componentDidMount() {
     this.loadDashboard(this.props.dashboardId);
+
+    const throttleParameterWidgetStickiness = _.throttle(
+      () => updateParametersWidgetStickiness(this),
+      SCROLL_THROTTLE_INTERVAL,
+    );
+
+    window.addEventListener("scroll", throttleParameterWidgetStickiness, {
+      passive: true,
+    });
+    window.addEventListener("resize", throttleParameterWidgetStickiness, {
+      passive: true,
+    });
   }
 
   UNSAFE_componentWillReceiveProps(nextProps) {
@@ -92,6 +114,9 @@ export default class Dashboard extends Component {
 
   componentWillUnmount() {
     this.props.cancelFetchDashboardCardData();
+
+    window.removeEventListener("scroll", updateParametersWidgetStickiness);
+    window.removeEventListener("resize", updateParametersWidgetStickiness);
   }
 
   async loadDashboard(dashboardId) {
@@ -169,7 +194,11 @@ export default class Dashboard extends Component {
       isSharing,
     } = this.props;
 
-    const { error, showAddQuestionSidebar } = this.state;
+    const {
+      error,
+      isParametersWidgetSticky,
+      showAddQuestionSidebar,
+    } = this.state;
 
     const shouldRenderAsNightMode = isNightMode && isFullscreen;
     const dashboardHasCards = dashboard => dashboard.ordered_cards.length > 0;
@@ -209,9 +238,16 @@ export default class Dashboard extends Component {
             </HeaderContainer>
 
             <DashboardBody isEditingOrSharing={isEditing || isSharing}>
-              <ParametersAndCardsContainer>
+              <ParametersAndCardsContainer
+                innerRef={element =>
+                  (this.parametersAndCardsContainerRef = element)
+                }
+              >
                 {!isFullscreen && parametersWidget && (
-                  <ParametersWidgetContainer>
+                  <ParametersWidgetContainer
+                    innerRef={element => (this.parametersWidgetRef = element)}
+                    isSticky={isParametersWidgetSticky}
+                  >
                     {parametersWidget}
                   </ParametersWidgetContainer>
                 )}

frontend/src/metabase/dashboard/components/Dashboard/Dashboard.styled.js

@@ -75,8 +75,19 @@ export const ParametersAndCardsContainer = styled.div`
 export const ParametersWidgetContainer = styled(FullWidthContainer)`
   align-items: flex-start;
   background-color: ${color("bg-light")};
+  border-bottom: 1px solid ${color("bg-light")};
   display: flex;
   flex-direction: column;
   padding-top: ${space(2)};
   padding-bottom: ${space(1)};
+  z-index: 4;
+
+  ${({ isSticky }) =>
+    isSticky &&
+    css`
+      border-bottom: 1px solid ${color("border")};
+      position: fixed;
+      top: 0;
+      left: 0;
+    `}
 `;

frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js

+export const updateParametersWidgetStickiness = dashboard => {
+  initializeWidgetOffsetTop(dashboard);
+
+  const shouldBeSticky = checkIfParametersWidgetShouldBeSticky(dashboard);
+
+  const shouldToggleStickiness = checkIfShouldToggleStickiness(
+    dashboard,
+    shouldBeSticky,
+  );
+
+  if (shouldToggleStickiness) {
+    updateParametersAndCardsContainerStyle(dashboard, shouldBeSticky);
+
+    dashboard.setState({
+      isParametersWidgetSticky: shouldBeSticky,
+    });
+  }
+};
+
+const initializeWidgetOffsetTop = dashboard => {
+  if (!dashboard.state.parametersWidgetOffsetTop) {
+    dashboard.setState({
+      parametersWidgetOffsetTop: dashboard.parametersWidgetRef.offsetTop,
+    });
+  }
+};
+
+const checkIfShouldToggleStickiness = (dashboard, shouldBeSticky) => {
+  const { isParametersWidgetSticky } = dashboard.state;
+
+  return shouldBeSticky !== isParametersWidgetSticky;
+};
+
+const checkIfParametersWidgetShouldBeSticky = dashboard => {
+  const offsetTop =
+    dashboard.state.parametersWidgetOffsetTop ||
+    dashboard.parametersWidgetRef.offsetTop;
+
+  return window.scrollY >= offsetTop;
+};
+
+const updateParametersAndCardsContainerStyle = (dashboard, shouldBeSticky) => {
+  const { offsetHeight } = dashboard.parametersWidgetRef;
+
+  const paddingTop = shouldBeSticky ? offsetHeight + "px" : "0";
+
+  dashboard.parametersAndCardsContainerRef.style.paddingTop = paddingTop;
+};

frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.unit.spec.js

+import { updateParametersWidgetStickiness } from "./stickyParameters";
+
+it("initializes parametersWidgetOffsetTop", () => {
+  const offsetTop = 100;
+  const setState = jest.fn();
+
+  const dashboard = {
+    parametersWidgetRef: { offsetTop },
+    parametersAndCardsContainerRef: { style: {} },
+    state: {},
+    setState,
+  };
+
+  updateParametersWidgetStickiness(dashboard);
+
+  expect(setState).toHaveBeenCalledWith({
+    parametersWidgetOffsetTop: offsetTop,
+  });
+});
+
+it("makes filters sticky with enough scrolling down", () => {
+  const offsetTop = 100;
+  const setState = jest.fn();
+
+  global.window.scrollY = offsetTop + 1;
+
+  const dashboard = {
+    parametersWidgetRef: { offsetTop },
+    parametersAndCardsContainerRef: { style: {} },
+    state: {},
+    setState,
+  };
+
+  updateParametersWidgetStickiness(dashboard);
+
+  expect(setState).toHaveBeenCalledWith({
+    isParametersWidgetSticky: true,
+  });
+});
+
+it("makes filters unsticky with enough scrolling up", () => {
+  const offsetTop = 100;
+  const setState = jest.fn();
+
+  global.window.scrollY = offsetTop - 1;
+
+  const dashboard = {
+    parametersWidgetRef: { offsetTop },
+    parametersAndCardsContainerRef: { style: {} },
+    state: {},
+    setState,
+  };
+
+  updateParametersWidgetStickiness(dashboard);
+
+  expect(setState).toHaveBeenCalledWith({
+    isParametersWidgetSticky: false,
+  });
+});
+
+it("keeps filters sticky with enough scrolling down and already sticky", () => {
+  const offsetTop = 100;
+  const setState = jest.fn();
+
+  global.window.scrollY = offsetTop + 1;
+
+  const dashboard = {
+    parametersWidgetRef: { offsetTop },
+    parametersAndCardsContainerRef: { style: {} },
+    state: { isParametersWidgetSticky: true, parametersWidgetOffsetTop: 100 },
+    setState,
+  };
+
+  updateParametersWidgetStickiness(dashboard);
+
+  expect(setState).not.toHaveBeenCalled();
+});
+
+it("keeps filters not sticky with enough scrolling up and already not sticky", () => {
+  const offsetTop = 100;
+  const setState = jest.fn();
+
+  global.window.scrollY = offsetTop - 1;
+
+  const dashboard = {
+    parametersWidgetRef: { offsetTop },
+    parametersAndCardsContainerRef: { style: {} },
+    state: { isParametersWidgetSticky: false, parametersWidgetOffsetTop: 100 },
+    setState,
+  };
+
+  updateParametersWidgetStickiness(dashboard);
+
+  expect(setState).not.toHaveBeenCalled();
+});

frontend/src/metabase/parameters/components/ParameterWidget.jsx

@@ -28,15 +28,21 @@ export default class ParameterWidget extends Component {
   };
 
   renderPopover(value, setValue, placeholder, isFullscreen) {
-    const { parameter, editingParameter, commitImmediately } = this.props;
-    const isEditingParameter = !!(
-      editingParameter && editingParameter.id === parameter.id
-    );
+    const {
+      dashboard,
+      parameter,
+      editingParameter,
+      commitImmediately,
+      parameters,
+    } = this.props;
+
+    const isEditingParameter = editingParameter?.id === parameter.id;
+
     return (
       <ParameterValueWidget
         parameter={parameter}
-        parameters={this.props.parameters}
-        dashboard={this.props.dashboard}
+        parameters={parameters}
+        dashboard={dashboard}
         name={name}
         value={value}
         setValue={setValue}

frontend/test/metabase-visual/dashboard/parameters-widget.cy.spec.js

+import { restore } from "__support__/e2e/cypress";
+import { SAMPLE_DATASET } from "__support__/e2e/cypress_sample_dataset";
+
+const { PRODUCTS_ID } = SAMPLE_DATASET;
+
+const questionDetails = {
+  query: { "source-table": PRODUCTS_ID },
+};
+
+const filter = {
+  name: "Category",
+  slug: "category",
+  id: "ad1c877e",
+  type: "category",
+};
+
+const filters = new Array(12).fill(filter);
+
+describe("visual tests > dashboard > parameters widget", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+
+    cy.createQuestionAndDashboard({ questionDetails }).then(
+      ({ body: oldCard }) => {
+        const { dashboard_id } = oldCard;
+
+        cy.request("PUT", `/api/dashboard/${dashboard_id}`, {
+          parameters: filters,
+        });
+
+        const updatedSize = {
+          sizeX: 12,
+          sizeY: 32,
+        };
+
+        cy.editDashboardCard(oldCard, updatedSize);
+
+        cy.visit(`/dashboard/${dashboard_id}`);
+      },
+    );
+  });
+
+  it("is sticky in view mode", () => {
+    cy.findByText("test question");
+
+    cy.scrollTo(0, 264);
+
+    cy.percySnapshot();
+  });
+});

frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js

@@ -216,14 +216,16 @@ describe("scenarios > dashboard", () => {
   });
 
   it("should display column options for cross-filter (metabase#14473)", () => {
-    cy.createNativeQuestion({
+    const questionDetails = {
       name: "14473",
       native: { query: "SELECT COUNT(*) FROM PRODUCTS", "template-tags": {} },
-    }).then(({ body: { id: QUESTION_ID } }) => {
-      cy.createDashboard().then(({ body: { id: DASHBOARD_ID } }) => {
+    };
+
+    cy.createNativeQuestionAndDashboard({ questionDetails }).then(
+      ({ body: { dashboard_id } }) => {
         cy.log("Add 4 filters to the dashboard");
 
-        cy.request("PUT", `/api/dashboard/${DASHBOARD_ID}`, {
+        cy.request("PUT", `/api/dashboard/${dashboard_id}`, {
           parameters: [
             { name: "ID", slug: "id", id: "729b6456", type: "id" },
             { name: "ID 1", slug: "id_1", id: "bb20f59e", type: "id" },
@@ -242,14 +244,9 @@ describe("scenarios > dashboard", () => {
           ],
         });
 
-        cy.log("Add previously created question to the dashboard");
-        cy.request("POST", `/api/dashboard/${DASHBOARD_ID}/cards`, {
-          cardId: QUESTION_ID,
-        });
-
-        cy.visit(`/dashboard/${DASHBOARD_ID}`);
-      });
-    });
+        cy.visit(`/dashboard/${dashboard_id}`);
+      },
+    );
 
     // Add cross-filter click behavior manually
     cy.icon("pencil").click();
@@ -460,6 +457,6 @@ function assertScrollBarExists() {
     const bodyWidth = $body[0].getBoundingClientRect().width;
     cy.window()
       .its("innerWidth")
-      .should("be.gt", bodyWidth);
+      .should("be.gte", bodyWidth);
   });
 }