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(ui5-dynamic-page): correct header snap/pin inconsistency at page top #11098

Merged
merged 2 commits into from
Mar 21, 2025

Conversation

NakataCode
Copy link
Contributor

Problem:
Several issues occurred with the header snap/pin interaction:

  1. When the header was pinned and then snapped, it would glitch into an inconsistent state appearing both snapped and unsnapped simultaneously
  2. After unpinning a previously pinned header, attempting to snap it would cause the same visual inconsistency
  3. Pinned headers that were snapped remained in a pinned state, causing unexpected behavior

Solution:

  • Modified the _toggleHeader method to properly detect when a pinned header is being snapped and ensure it unpins automatically. Also improved the onPinClick method to better handle unpinning when at the top of the page by checking the scroll position and updating the showHeaderInStickArea property accordingly.

Fixes: #11088

@NakataCode NakataCode marked this pull request as ready for review March 14, 2025 10:38
@@ -372,6 +374,17 @@ class DynamicPage extends UI5Element {
const headerHeight = this.dynamicPageHeader?.getBoundingClientRect().height || 0;
const currentScrollTop = this.scrollContainer!.scrollTop;

if (!this._headerSnapped && this.headerPinned) {
this.headerPinned = false;
this.fireDecoratorEvent("pin-button-toggle");
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure it's correct to fire pin button toggle without actually clicking on the button ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe firing the event here makes sense. When we programmatically change the pin state (headerPinned = false), it helps notify the UI and any listeners about this state change, similar to when a user clicks the pin button. The event helps ensure the visual state stays in sync with the internal state.

@plamenivanov91 plamenivanov91 self-requested a review March 21, 2025 08:37
@NakataCode NakataCode merged commit 43fe1e0 into main Mar 21, 2025
12 checks passed
@NakataCode NakataCode deleted the dynamic-page-pinned-header branch March 21, 2025 08:46
@ui5-webcomponents-bot
Copy link
Collaborator

🎉 This PR is included in version v2.9.0-rc.2 🎉

The release is available on v2.9.0-rc.2

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

[ui5-dynamic-page]: Header snap/pin glitches when activated at top of page
3 participants