Skip to content

Commit 58a89eb

Browse files
committed
fix(plugin-chart-table): decouple sticky-header scrollbar measurement from the shared getScrollBarSize util
rusackas pointed out that getScrollBarSize() is also used by TableChart.tsx's resize-twitch guard, which doesn't render the custom scrollbar styling, so making the shared measurement return the custom 8px width could throw off that unrelated threshold. Split it: getScrollBarSize() goes back to measuring the plain native scrollbar, and a new getCustomScrollBarSize() (used only by useSticky.tsx) measures with the custom ::-webkit-scrollbar styling applied.
1 parent ae39288 commit 58a89eb

3 files changed

Lines changed: 67 additions & 31 deletions

File tree

superset-frontend/plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ import {
3131
} from 'react';
3232
import { TableInstance, Hooks } from 'react-table';
3333
import { useTheme, css } from '@apache-superset/core/theme';
34-
import getScrollBarSize, {
34+
import {
3535
CUSTOM_SCROLLBAR_SIZE,
36+
getCustomScrollBarSize,
3637
} from '../utils/getScrollBarSize';
3738
import needScrollBar from '../utils/needScrollBar';
3839
import useMountedMemo from '../utils/useMountedMemo';
@@ -167,7 +168,7 @@ function StickyWrap({
167168
const scrollFooterRef = useRef<HTMLDivElement>(null); // fixed footer
168169
const scrollBodyRef = useRef<HTMLDivElement>(null); // main body
169170

170-
const scrollBarSize = getScrollBarSize();
171+
const scrollBarSize = getCustomScrollBarSize();
171172
const { bodyHeight, columnWidths, hasVerticalScroll } = sticky;
172173
const needSizer =
173174
!columnWidths ||

superset-frontend/plugins/plugin-chart-table/src/DataTable/utils/getScrollBarSize.ts

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,45 +17,69 @@
1717
* under the License.
1818
*/
1919
// Width/height of the custom `::-webkit-scrollbar` applied to the sticky
20-
// body and header sizer in useSticky.tsx. Shared here so the measured
21-
// scrollbar size always matches what actually renders.
20+
// body and header sizer in useSticky.tsx.
2221
export const CUSTOM_SCROLLBAR_SIZE = 8;
2322

2423
let cached: number | undefined;
24+
let cachedCustom: number | undefined;
2525

2626
const css = (x: TemplateStringsArray) => x.join('\n');
2727

28+
function measureScrollBarSize(probeClassName?: string) {
29+
const inner = document.createElement('div');
30+
const outer = document.createElement('div');
31+
// Custom scrollbars are only styleable via a stylesheet rule, since
32+
// inline styles can't express `::-webkit-scrollbar` pseudo-elements.
33+
const style = probeClassName ? document.createElement('style') : undefined;
34+
if (style && probeClassName) {
35+
style.textContent = `.${probeClassName}::-webkit-scrollbar { width: ${CUSTOM_SCROLLBAR_SIZE}px; height: ${CUSTOM_SCROLLBAR_SIZE}px; }`;
36+
inner.className = probeClassName;
37+
document.head.append(style);
38+
}
39+
inner.style.cssText = css`
40+
width: auto;
41+
height: 100%;
42+
overflow: scroll;
43+
`;
44+
outer.style.cssText = css`
45+
position: absolute;
46+
visibility: hidden;
47+
overflow: hidden;
48+
width: 100px;
49+
height: 50px;
50+
`;
51+
outer.append(inner);
52+
document.body.append(outer);
53+
const size = outer.clientWidth - inner.clientWidth;
54+
outer.remove();
55+
style?.remove();
56+
return size;
57+
}
58+
59+
// Measures the browser/OS native scrollbar width. Used anywhere an
60+
// un-styled scrollable region's real footprint matters (e.g. the chart's
61+
// outer resize-twitch guard in TableChart.tsx).
2862
export default function getScrollBarSize(forceRefresh = false) {
2963
if (typeof document === 'undefined') {
3064
return 0;
3165
}
3266
if (cached === undefined || forceRefresh) {
33-
const probeClassName = 'superset-scrollbar-size-probe';
34-
const inner = document.createElement('div');
35-
const outer = document.createElement('div');
36-
const style = document.createElement('style');
37-
// Custom scrollbars are only styleable via a stylesheet rule, since
38-
// inline styles can't express `::-webkit-scrollbar` pseudo-elements.
39-
style.textContent = `.${probeClassName}::-webkit-scrollbar { width: ${CUSTOM_SCROLLBAR_SIZE}px; height: ${CUSTOM_SCROLLBAR_SIZE}px; }`;
40-
inner.className = probeClassName;
41-
inner.style.cssText = css`
42-
width: auto;
43-
height: 100%;
44-
overflow: scroll;
45-
`;
46-
outer.style.cssText = css`
47-
position: absolute;
48-
visibility: hidden;
49-
overflow: hidden;
50-
width: 100px;
51-
height: 50px;
52-
`;
53-
outer.append(inner);
54-
document.head.append(style);
55-
document.body.append(outer);
56-
cached = outer.clientWidth - inner.clientWidth;
57-
outer.remove();
58-
style.remove();
67+
cached = measureScrollBarSize();
5968
}
6069
return cached;
6170
}
71+
72+
// Measures the scrollbar width as it actually renders with the custom
73+
// `::-webkit-scrollbar` styling applied (see scrollBarStyles in
74+
// useSticky.tsx). On browsers that don't support `::-webkit-scrollbar`
75+
// (e.g. Firefox) this equals the native scrollbar width. Only use this for
76+
// elements that carry that same custom styling.
77+
export function getCustomScrollBarSize(forceRefresh = false) {
78+
if (typeof document === 'undefined') {
79+
return 0;
80+
}
81+
if (cachedCustom === undefined || forceRefresh) {
82+
cachedCustom = measureScrollBarSize('superset-scrollbar-size-probe');
83+
}
84+
return cachedCustom;
85+
}

superset-frontend/plugins/plugin-chart-table/test/DataTable/utils/getScrollBarSize.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,24 @@
1818
*/
1919
import getScrollBarSize, {
2020
CUSTOM_SCROLLBAR_SIZE,
21+
getCustomScrollBarSize,
2122
} from '../../../src/DataTable/utils/getScrollBarSize';
2223

23-
test('measures the scrollbar probe using the shared custom scrollbar size', () => {
24+
test('getScrollBarSize measures the native scrollbar without any custom styling probe', () => {
2425
const appendSpy = jest.spyOn(document.head, 'append');
2526

2627
getScrollBarSize(true);
2728

29+
expect(appendSpy).not.toHaveBeenCalled();
30+
31+
appendSpy.mockRestore();
32+
});
33+
34+
test('getCustomScrollBarSize measures the probe using the shared custom scrollbar size', () => {
35+
const appendSpy = jest.spyOn(document.head, 'append');
36+
37+
getCustomScrollBarSize(true);
38+
2839
const styleEl = appendSpy.mock.calls
2940
.map(args => args[0])
3041
.find((node): node is HTMLStyleElement => node instanceof HTMLStyleElement);

0 commit comments

Comments
 (0)