Skip to content

fix(ui5-dynamic-page): hide pin button on mobile devices#13335

Merged
NakataCode merged 2 commits intomainfrom
dynamic-page-hide-pin-mobile
Mar 26, 2026
Merged

fix(ui5-dynamic-page): hide pin button on mobile devices#13335
NakataCode merged 2 commits intomainfrom
dynamic-page-hide-pin-mobile

Conversation

@NakataCode
Copy link
Contributor

@NakataCode NakataCode commented Mar 26, 2026

The pin/unpin button is now automatically hidden on mobile devices in accordance with SAP Fiori guidelines, which state that the Pin Header action should not be provided on small screens as the pinned header would take up too much screen real estate.

  • Additionally, introduced PAGE_HEIGHT constant in DynamicPage.mobile.cy to reduce repetition of hardcoded "600px" values.

Fixes #13320

@ui5-webcomponents-bot
Copy link
Collaborator

ui5-webcomponents-bot commented Mar 26, 2026

🧹 Preview deployment cleaned up: https://pr-13335--ui5-webcomponents.netlify.app

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview March 26, 2026 13:35 Inactive
@kgogov kgogov self-requested a review March 26, 2026 13:35
@kgogov
Copy link
Contributor

kgogov commented Mar 26, 2026

Code review

Found 1 issue:

  1. Test uses wrong CSS class selector, making it a no-op. The new test asserts .find(".ui5-dynamic-page-header-action-pin-button").should("not.exist"), but the class ui5-dynamic-page-header-action-pin-button does not exist anywhere in the codebase. The actual class on the pin button ToggleButton is ui5-dynamic-page-header-action-pin (no -button suffix). Because the selector never matches anything, the should("not.exist") assertion passes vacuously in all cases -- the test would pass even if the pin button were fully visible on mobile. The correct selector is .ui5-dynamic-page-header-action-pin, consistent with the desktop tests and the source code.

Wrong selector in test:

.shadow()
.find(".ui5-dynamic-page-header-action-pin-button")
.should("not.exist");

Actual class in template:

<ToggleButton
class="ui5-dynamic-page-header-action ui5-dynamic-page-header-action-pin"
onClick={this.onPinClick}

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview March 26, 2026 15:49 Inactive
@NakataCode NakataCode merged commit 359fe49 into main Mar 26, 2026
14 checks passed
@NakataCode NakataCode deleted the dynamic-page-hide-pin-mobile branch March 26, 2026 19:41
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview March 26, 2026 19:42 Inactive
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.

[Dynamic page]: Pin/unpin button should not be available on mobile devices

3 participants