Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(useElementVisibility): adjust threshold to 0 to fix visibility issue with large element #3308

Merged
merged 1 commit into from Aug 25, 2023

Conversation

erikkkwu
Copy link
Contributor

@erikkkwu erikkkwu commented Aug 10, 2023

…sue with large element

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.
⚠️ Slowing down new functions

Warning: Slowing down new functions

As the VueUse audience continues to grow, we have been inundated with an overwhelming number of feature requests and pull requests. As a result, maintaining the project has become increasingly challenging and has stretched our capacity to its limits. As such, in the near future, we may need to slow down our acceptance of new features and prioritize the stability and quality of existing functions. Please note that new features for VueUse may not be accepted at this time. If you have any new ideas, we suggest that you first incorporate them into your own codebase, iterate on them to suit your needs, and assess their generalizability. If you strongly believe that your ideas are beneficial to the community, you may submit a pull request along with your use cases, and we would be happy to review and discuss them. Thank you for your understanding.


Description

fix #3305
large element cause intersectionRatio to fall below the default threshold 0.1, making elementIsVisible false (e.g., viewport height 900 / target height 9001).

Additional context


🤖 Generated by Copilot at 65d7dfa

Added threshold option to useElementVisibility hook to support elements with zero dimensions. This option controls how much of the element's area needs to be visible before the hook reports it as visible.

🤖 Generated by Copilot at 65d7dfa

  • Add threshold option to IntersectionObserver constructor to detect visibility of zero-sized elements (link)

Copy link

@samuveth samuveth left a comment

Choose a reason for hiding this comment

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

tACK

@antfu antfu added this pull request to the merge queue Aug 25, 2023
Merged via the queue into vueuse:main with commit 429edda Aug 25, 2023
5 checks passed
@rikzwarthoff
Copy link

@erikkkwu @antfu @samuveth, Hi guys, this parameter is now hard coded, whilst it should be passed through as an option imo. We've had issues with 0 threshold, I actually need it to be 0.1

@edtorba
Copy link
Contributor

edtorba commented Nov 15, 2023

@erikkkwu @antfu @samuveth, Hi guys, this parameter is now hard coded, whilst it should be passed through as an option imo. We've had issues with 0 threshold, I actually need it to be 0.1

I had the exact same issue. As a workaround, I moved to using useIntersectionObserver. However, I am considering spinning up a PR to allow its definition via an option.

@erikkkwu
Copy link
Contributor Author

Hello @rikzwarthoff @edtorba, I would like to inquire about the specific situations in which you encounter issues. Indeed, if you do face problems, I agree that it should be possible to turn it into an option

@rikzwarthoff
Copy link

rikzwarthoff commented Nov 21, 2023

Hi @erikkkwu, we have the following situation. A div wrapped around a table, which has multiple tbody elements, each of those tbody elements has a useElementVisibility where the scrollTarget is the div. Now the table has an after element with calculated height so you can always scroll the last tbody to the top. The first row of each tbody is sticky, my guess that the problem would the there.

I can sent you the url privately, if you would want it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useInfiniteScroll: randomly stops working
5 participants