Skip to content

Commit 2fac61c

Browse files
jcheng5schloerke
andauthored
bug(data frame): Fix scrolling in fillable cards (posit-dev#1550)
Co-authored-by: Barret Schloerke <barret@posit.co>
1 parent 2fbc800 commit 2fac61c

File tree

4 files changed

+32
-16
lines changed

4 files changed

+32
-16
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6161

6262
* Fixed #1440: When a Shiny Express app with a `www/` subdirectory was deployed to shinyapps.io or a Connect server, it would not start correctly. (#1442)
6363

64-
* Fixed #1498: Update table related TypeScript dependencies to their latest versions. This fixed an issue where the Row Virtualizer would scroll to the end when hidden. This would cause the DOM to update numerous times, locking up the browser tab for multiple seconds. #1524
64+
* Fixed #1498: Update table related TypeScript dependencies to their latest versions. This fixed an issue where the Row Virtualizer would scroll to the end when hidden. This would cause the DOM to update numerous times, locking up the browser tab for multiple seconds. (#1524, #1550)
6565

6666
* The return type for the data frame patch function now returns a list of `render.CellPatch` objects (which support `htmltools.TagNode` for the `value` attribute). These values will be set inside the data frame's `.data_view()` result. This also means that `.cell_patches()` will be a list of `render.CellPatch` objects. (#1526)
6767

js/data-frame/index.tsx

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -648,13 +648,29 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
648648

649649
const headerRowCount = table.getHeaderGroups().length;
650650

651-
// Assume we're scrolling until proven otherwise
652-
let scrollingClass = tableData.length > 0 ? "scrolling" : "";
653-
const scrollHeight = containerRef.current?.scrollHeight;
654-
const clientHeight = containerRef.current?.clientHeight;
655-
if (scrollHeight && clientHeight && scrollHeight <= clientHeight) {
656-
scrollingClass = "";
657-
}
651+
// Maintain the .scrolling class: present if the amount of data in the table causes
652+
// vertical overflow, absent if not.
653+
useLayoutEffect(() => {
654+
// If no data, we're definitely not scrolling. Otherwise, we need to test.
655+
let scrolling = tableData.length > 0;
656+
if (scrolling) {
657+
// We need to add .scrolling before comparing scrollHeight/clientHeight. If not,
658+
// then if there's already a scrollworthy amount of data, we might get stuck in
659+
// non-scrolling state because the clientHeight is expanding within a fixed
660+
// container. (See https://github.com/posit-dev/py-shiny/issues/1549)
661+
containerRef.current?.classList.add("scrolling");
662+
const scrollHeight = containerRef.current?.scrollHeight;
663+
const clientHeight = containerRef.current?.clientHeight;
664+
if (scrollHeight && clientHeight && scrollHeight <= clientHeight) {
665+
scrolling = false;
666+
}
667+
}
668+
containerRef.current?.classList.toggle("scrolling", scrolling);
669+
}, [
670+
tableData.length,
671+
containerRef.current?.scrollHeight,
672+
containerRef.current?.clientHeight,
673+
]);
658674

659675
const makeHeaderKeyDown =
660676
(column: Column<unknown[], unknown>) => (event: React.KeyboardEvent) => {
@@ -665,7 +681,7 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
665681

666682
const measureEl = useVirtualizerMeasureWorkaround(rowVirtualizer);
667683

668-
let className = `shiny-data-grid ${containerClass} ${scrollingClass}`;
684+
let className = `shiny-data-grid ${containerClass}`;
669685
if (fill) {
670686
className += " html-fill-item";
671687
}

0 commit comments

Comments
 (0)