Skip to content

Commit

Permalink
fix(listbox): unexpected scrollShadow on virtualized listbox (#4784)
Browse files Browse the repository at this point in the history
* fix(listbox): add scroll height & scroll top to listbox

* fix(use-data-scroll-overflow): handle scrollHeight & scrollTop in virtualization

* chore(changeset): add changeset
  • Loading branch information
wingkwong authored Feb 5, 2025
1 parent eb92904 commit f7c2be0
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
6 changes: 6 additions & 0 deletions .changeset/ninety-flowers-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@heroui/use-data-scroll-overflow": patch
"@heroui/listbox": patch
---

fixed unexpected scrollShadow on virtualized listbox (#4553)
17 changes: 11 additions & 6 deletions packages/components/listbox/src/virtualized-listbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ const VirtualizedListbox = (props: Props) => {

const virtualItems = rowVirtualizer.getVirtualItems();

/* Here we need the base props for scroll shadow, contains the className (scrollbar-hide and scrollshadow config based on the user inputs on select props) */
const virtualScrollHeight = rowVirtualizer.getTotalSize();

// Here we need the base props for scroll shadow,
// contains the className (scrollbar-hide and scrollshadow config based on the user inputs on select props)
const {getBaseProps: getBasePropsScrollShadow} = useScrollShadow({...scrollShadowProps});

const renderRow = (virtualItem: VirtualItem) => {
Expand Down Expand Up @@ -162,14 +165,19 @@ const VirtualizedListbox = (props: Props) => {
return listboxItem;
};

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const [scrollState, setScrollState] = useState({
isTop: false,
isBottom: true,
isMiddle: false,
});

const content = (
<Component {...getListProps()}>
<Component
{...getListProps()}
data-virtual-scroll-height={virtualScrollHeight}
data-virtual-scroll-top={parentRef?.current?.scrollTop}
>
{!state.collection.size && !hideEmptyContent && (
<li>
<div {...getEmptyContentProps()} />
Expand All @@ -178,9 +186,6 @@ const VirtualizedListbox = (props: Props) => {
<div
{...filterDOMProps(getBasePropsScrollShadow())}
ref={parentRef}
data-bottom-scroll={scrollState.isTop}
data-top-bottom-scroll={scrollState.isMiddle}
data-top-scroll={scrollState.isBottom}
style={{
height: maxListboxHeight,
overflow: "auto",
Expand All @@ -192,7 +197,7 @@ const VirtualizedListbox = (props: Props) => {
{listHeight > 0 && itemHeight > 0 && (
<div
style={{
height: `${rowVirtualizer.getTotalSize()}px`,
height: `${virtualScrollHeight}px`,
width: "100%",
position: "relative",
}}
Expand Down
19 changes: 14 additions & 5 deletions packages/hooks/use-data-scroll-overflow/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,22 @@ export function useDataScrollOverflow(props: UseDataScrollOverflowProps = {}) {
{type: "horizontal", prefix: "left", suffix: "right"},
];

const listbox = el.querySelector('ul[data-slot="list"]');

// in virtualized listbox, el.scrollHeight is the height of the visible listbox
const scrollHeight = +(
listbox?.getAttribute("data-virtual-scroll-height") ?? el.scrollHeight
);

// in virtualized listbox, el.scrollTop is always 0
const scrollTop = +(listbox?.getAttribute("data-virtual-scroll-top") ?? el.scrollTop);

for (const {type, prefix, suffix} of directions) {
if (overflowCheck === type || overflowCheck === "both") {
const hasBefore = type === "vertical" ? el.scrollTop > offset : el.scrollLeft > offset;
const hasBefore = type === "vertical" ? scrollTop > offset : el.scrollLeft > offset;
const hasAfter =
type === "vertical"
? el.scrollTop + el.clientHeight + offset < el.scrollHeight
? scrollTop + el.clientHeight + offset < scrollHeight
: el.scrollLeft + el.clientWidth + offset < el.scrollWidth;

setAttributes(type, hasBefore, hasAfter, prefix, suffix);
Expand All @@ -132,8 +142,7 @@ export function useDataScrollOverflow(props: UseDataScrollOverflowProps = {}) {
};

// auto
checkOverflow();
el.addEventListener("scroll", checkOverflow);
el.addEventListener("scroll", checkOverflow, true);

// controlled
if (visibility !== "auto") {
Expand All @@ -152,7 +161,7 @@ export function useDataScrollOverflow(props: UseDataScrollOverflowProps = {}) {
}

return () => {
el.removeEventListener("scroll", checkOverflow);
el.removeEventListener("scroll", checkOverflow, true);
clearOverflow();
};
}, [...updateDeps, isEnabled, visibility, overflowCheck, onVisibilityChange, domRef]);
Expand Down

0 comments on commit f7c2be0

Please sign in to comment.