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

Filter out hidden elements in sirius.addElementVisibleListener #1457

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

jmuscireum
Copy link
Contributor

Description

Hidden elements have a bounding client rect at the top of the viewport and thus the listener is triggered immediately after the page is loaded.

This adds a simple check for visibility by checking the width and height of the bounding client rect.

Additional Notes

  • This PR fixes or works on following ticket(s): SE-13968

Checklist

  • Code change has been tested and works locally
  • Code was formatted via IntelliJ and follows SonarLint & best practices
  • Patch Tasks: Is local execution of Patch Tasks necessary? If so, please also mark the PR with the tag.

Hidden elements have a bounding client rect at the top of the viewport and thus the listener is triggered immediately after the page is loaded.

This adds a simple check for visibility by checking the width and height of the bounding client rect.

Fixes: SE-13968
@jmuscireum jmuscireum added 🐛 Bugfix Contains only a small fix for an existing bug 👶🏻 Trivial Easy to review labels Sep 5, 2024
The observed performance impact (as checked by the Chrome profiler) was minimal.

Fixes: SE-13968
@jmuscireum jmuscireum requested a review from sabieber September 5, 2024 15:15
Copy link
Member

@sabieber sabieber left a comment

Choose a reason for hiding this comment

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

Thx 👌🏻

@ymo-sci ymo-sci merged commit 2dfc2f7 into develop Sep 11, 2024
3 checks passed
@ymo-sci ymo-sci deleted the feature/jmu/SE-13968-hidden-elements-are-not-visible branch September 11, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bugfix Contains only a small fix for an existing bug 👶🏻 Trivial Easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants