Skip to content

fix(design-system): useIsVisible observer look-ahead - #3210

Merged
JammingBen merged 1 commit into
mainfrom
fix/visible-observer-lookahead
Sep 2, 2026
Merged

JammingBen merged 1 commit into
mainfrom
fix/visible-observer-lookahead

Conversation

@JammingBen

@JammingBen JammingBen commented Aug 24, 2026 •

Copy link
Copy Markdown
Member

Fixes the look-ahead of the useIsVisible composable by passing it the parent scroll container as root property and creating the observer lazily to ensure the target element is mounted.

You can test this by adjusting your browser window size so it fits e.g. 15 tiles in the file list. Now open a folder with >30 images and look at the network tab. It should load ~30 thumbnails because of the fixed look-ahead, which is now about one viewport.

Breaking for devs

The root prop of the useIsVisible composable is mandatory now.

fixes #3168

@JammingBen JammingBen self-assigned this Aug 24, 2026
@JammingBen
JammingBen marked this pull request as ready for review August 24, 2026 13:10
root,
mode = 'show',
rootMargin = '100px',
rootMargin = '100% 0%',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rootMargin = '100% 0%',
rootMargin = '100% 0',

css 0 values typically don't hold a unit

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this doesn't work here. It's not a css value, it's an option of the IntersectionObserver.

Comment thread packages/web-pkg/src/composables/filesList/useFilesViewScrollContainer.ts Outdated

const observerTarget = useTemplateRef<InstanceType<typeof OcCard>>('observerTarget')
const observerTargetElement = computed<HTMLElement>(() => unref(observerTarget)?.$el)
const scrollContainer = useFilesViewScrollContainer()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait, why don't you pass down the scrollcontainer from the ResourceTiles component like you did with the table?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be passed to useIsVisible, not to the component. The problem with the table is that useIsVisible lives deep inside the trs, hence we need to pass it down further in that case.


export const useIsVisible = ({
target,
root,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strictly speaking, since root is required this is a breaking change for the design-system

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thing that a major release is around the corner 🤓

Fixes the look-ahead of the useIsVisible composable by passing it the parent
scroll container as root property and creating the observer lazily to ensure
the target element is mounted.

Breaking for devs: the root property on useIsVisible is mandatory now.
@JammingBen
JammingBen force-pushed the fix/visible-observer-lookahead branch from a4ad393 to 948d611 Compare September 2, 2026 06:53
@JammingBen
JammingBen merged commit 3c44b70 into main Sep 2, 2026
31 checks passed
@JammingBen
JammingBen deleted the fix/visible-observer-lookahead branch September 2, 2026 07:16
openclouders pushed a commit that referenced this pull request Sep 2, 2026
…head

fix(design-system): useIsVisible observer look-ahead
@openclouders openclouders mentioned this pull request Sep 2, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File list lazy loading has no look-ahead: rootMargin is ineffective

2 participants