Skip to content
Snippets Groups Projects
Unverified Commit 2fe7b83b authored by Diego Rocha's avatar Diego Rocha Committed by GitHub
Browse files

Fix dashboard scroll jank when there are no filters (#32245)

* Fix dashboard scroll jank is some cases

The behavior defined by stickyParameters.js was updating the dashboard's parametersWidgetOffsetTop state with undefined at every scroll event, causing jank. This was caused by mistakenly assuming that a dashboard component always has its parametersWidgetRef set to something other than an empty React ref, when in fact it can be null when the dashboard has no filters. This changes also fix how a React ref is used to make parametersWidgetRef reference the HTMLElement the parameters widget.

* Remove initializeWidgetOffsetTop altogether
parent b01a41bb
Branches
Tags
No related merge requests found
......@@ -340,7 +340,7 @@ class Dashboard extends Component {
{shouldRenderParametersWidgetInViewMode && (
<ParametersWidgetContainer
data-testid="dashboard-parameters-widget-container"
ref={element => (this.parametersWidgetRef = element)}
ref={this.parametersWidgetRef}
isNavbarOpen={isNavbarOpen}
isSticky={isParametersWidgetSticky}
topNav={embedOptions?.top_nav}
......
......@@ -3,8 +3,6 @@ import { isSmallScreen, getMainElement } from "metabase/lib/dom";
export const MAXIMUM_PARAMETERS_FOR_STICKINESS = 6;
export const updateParametersWidgetStickiness = dashboard => {
initializeWidgetOffsetTop(dashboard);
const shouldBeSticky = checkIfParametersWidgetShouldBeSticky(dashboard);
const shouldToggleStickiness = checkIfShouldToggleStickiness(
......@@ -19,14 +17,6 @@ export const updateParametersWidgetStickiness = dashboard => {
}
};
const initializeWidgetOffsetTop = dashboard => {
if (!dashboard.state.parametersWidgetOffsetTop) {
dashboard.setState({
parametersWidgetOffsetTop: dashboard.parametersWidgetRef.offsetTop,
});
}
};
const checkIfShouldToggleStickiness = (dashboard, shouldBeSticky) => {
const { isParametersWidgetSticky } = dashboard.state;
......@@ -52,6 +42,12 @@ const checkIfParametersWidgetShouldBeSticky = dashboard => {
return getMainElement().scrollTop > offsetTop;
};
const getOffsetTop = dashboard =>
dashboard.state.parametersWidgetOffsetTop ||
dashboard.parametersWidgetRef.offsetTop;
const getOffsetTop = dashboard => {
const parametersWidget = dashboard.parametersWidgetRef.current;
if (parametersWidget) {
return parametersWidget.offsetTop;
} else {
return 0;
}
};
......@@ -8,31 +8,13 @@ function mockMainElementScroll(scrollTop) {
}
describe("updateParametersWidgetStickiness", () => {
it("initializes parametersWidgetOffsetTop", () => {
const setState = jest.fn();
mockMainElementScroll(0);
const dashboard = {
parametersWidgetRef: { offsetTop },
state: {},
setState,
};
updateParametersWidgetStickiness(dashboard);
expect(setState).toHaveBeenCalledWith({
parametersWidgetOffsetTop: offsetTop,
});
});
it("makes filters sticky with enough scrolling down", () => {
const setState = jest.fn();
mockMainElementScroll(offsetTop + 1);
const dashboard = {
parametersWidgetRef: { offsetTop },
parametersWidgetRef: { current: { offsetTop } },
state: {},
setState,
};
......@@ -50,7 +32,7 @@ describe("updateParametersWidgetStickiness", () => {
mockMainElementScroll(offsetTop - 1);
const dashboard = {
parametersWidgetRef: { offsetTop },
parametersWidgetRef: { current: { offsetTop } },
state: {},
setState,
};
......@@ -68,10 +50,9 @@ describe("updateParametersWidgetStickiness", () => {
mockMainElementScroll(offsetTop + 1);
const dashboard = {
parametersWidgetRef: { offsetTop },
parametersWidgetRef: { current: { offsetTop } },
state: {
isParametersWidgetSticky: true,
parametersWidgetOffsetTop: offsetTop,
},
setState,
};
......@@ -87,10 +68,9 @@ describe("updateParametersWidgetStickiness", () => {
mockMainElementScroll(offsetTop - 1);
const dashboard = {
parametersWidgetRef: { offsetTop },
parametersWidgetRef: { current: { offsetTop } },
state: {
isParametersWidgetSticky: false,
parametersWidgetOffsetTop: offsetTop,
},
setState,
};
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment