Skip to content

fix(ui5-button): adjust tooltips display #11268

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

Merged
merged 8 commits into from
Apr 23, 2025
Merged

fix(ui5-button): adjust tooltips display #11268

merged 8 commits into from
Apr 23, 2025

Conversation

unazko
Copy link
Contributor

@unazko unazko commented Apr 4, 2025

Issue:

  • The default icon tooltip is displayed over the ui5-button even though there is a text
    in the ui5-button's slot.

Solution:

  • The default icon tooltip should only be shown when the ui5-button is in icon-only mode
    according to the UX guidelines.

Fixes: #10824

@unazko unazko marked this pull request as ready for review April 7, 2025 10:21
@unazko unazko requested a review from didip1000 April 14, 2025 07:15
@unazko unazko requested a review from didip1000 April 16, 2025 08:58
@unazko unazko requested a review from nnaydenow April 23, 2025 09:16
@unazko unazko merged commit 9f0d907 into main Apr 23, 2025
12 checks passed
@ui5-webcomponents-bot
Copy link
Collaborator

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

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

Your semantic-release bot 📦🚀

@nnaydenow nnaydenow deleted the button_tooltip_display branch April 29, 2025 10:39
@unazko unazko requested a review from Copilot April 30, 2025 07:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adjusts the tooltip display behavior on ui5-button so that the default icon tooltip is only applied when the button is icon-only, addressing the UX issue described in #10824.

  • Removed the showTooltip property from the ButtonTemplate to simplify tooltip handling.
  • Updated Button.ts to only assign a default tooltip if the button is icon-only.
  • Extended tests in Button.html and Cypress specs to validate the new tooltip behavior.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/main/test/pages/Button.html Added buttons with varied configurations to test tooltip behavior.
packages/main/src/ButtonTemplate.tsx Removed the unused showTooltip prop in the icon rendering.
packages/main/src/Button.ts Updated tooltip assignment logic to honor icon-only mode when applying the default tooltip.
packages/main/cypress/specs/Button.cy.tsx Added a test to ensure that a tooltip is not rendered when text is present.
Comments suppressed due to low confidence (3)

packages/main/src/ButtonTemplate.tsx:50

  • The removal of the showTooltip prop simplifies the template and aligns with the updated tooltip logic. Consider verifying that any related unused properties or code paths are also cleaned up to improve maintainability.
showTooltip={this.showIconTooltip}

packages/main/cypress/specs/Button.cy.tsx:242

  • [nitpick] The test for ensuring no tooltip is displayed when text is present is good; however, consider using a more specific selector to target the intended button instance in case multiple buttons exist in the DOM.
cy.mount(<Button icon="home">Action</Button>);

packages/main/src/Button.ts:370

  • The updated conditional assignment of 'buttonTitle' ensures that the default tooltip is only applied when the button is in icon-only mode. Please verify that cases where no tooltip is provided (and the button is not icon-only) correctly result in 'buttonTitle' remaining undefined.
const defaultTooltip = await this.getDefaultTooltip();

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.

[SF_ACC][ui5-toolbar-button]: show unexpected tooltip for some icon button
4 participants