diff --git a/frontend/src/metabase/components/TokenField.jsx b/frontend/src/metabase/components/TokenField.jsx
index 83f888c72eee2312b82c8bcda6232714e36bfb8a..3c161d1f8870506312e73a04339d39b041896eb6 100644
--- a/frontend/src/metabase/components/TokenField.jsx
+++ b/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}
diff --git a/frontend/src/metabase/css/dashboard.css b/frontend/src/metabase/css/dashboard.css
index 37a03c4f785a8bcf21b4c9a18c4a144139af41d7..fb232e14351c16c90cfab148965fd2d1cdc467f8 100644
--- a/frontend/src/metabase/css/dashboard.css
+++ b/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;
}
diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx
index 3a666925444c40fe4a3af553fb6af79ae8c0ef44..c620c85adb05f179ec268374c506f519b3381e9d 100644
--- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx
+++ b/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>
)}
diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.styled.js b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.styled.js
index 6d162daff3589ea3d0c203a34be7911932b65f41..d96e26fce335bf7676dc7ff446e7e2e72e2fea1a 100644
--- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.styled.js
+++ b/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;
+ `}
`;
diff --git a/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js b/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js
new file mode 100644
index 0000000000000000000000000000000000000000..d4b61176a22e4e7b2d304c3409de84da3cc70449
--- /dev/null
+++ b/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.js
@@ -0,0 +1,48 @@
+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;
+};
diff --git a/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.unit.spec.js b/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.unit.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..b7fb94f8538da786055bc76ab4b24fd05ae3652b
--- /dev/null
+++ b/frontend/src/metabase/dashboard/components/Dashboard/stickyParameters.unit.spec.js
@@ -0,0 +1,95 @@
+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();
+});
diff --git a/frontend/src/metabase/parameters/components/ParameterWidget.jsx b/frontend/src/metabase/parameters/components/ParameterWidget.jsx
index c2bf020f7ddcd3ae4b6fc6bf9e25a248ffa955ef..080c65497c06585c23ad230f9cd6352b3d4b6cf0 100644
--- a/frontend/src/metabase/parameters/components/ParameterWidget.jsx
+++ b/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}
diff --git a/frontend/test/metabase-visual/dashboard/parameters-widget.cy.spec.js b/frontend/test/metabase-visual/dashboard/parameters-widget.cy.spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..8c75997eb94d1e98bd36012a5283f6479f7b3f1c
--- /dev/null
+++ b/frontend/test/metabase-visual/dashboard/parameters-widget.cy.spec.js
@@ -0,0 +1,51 @@
+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();
+ });
+});
diff --git a/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js b/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js
index 62b0d99a61d15e0e65db357d1e84cfb550ca42c3..56dc14d4584f469ecaf9606ccb1c00f476ec11be 100644
--- a/frontend/test/metabase/scenarios/dashboard/dashboard.cy.spec.js
+++ b/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);
});
}