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

[SF][RatingIndicator]: Updated title attribute on the ui5 component will not be reflected in the shadow dom #7264

Closed
1 task done
Jasonl157 opened this issue Jun 26, 2023 · 8 comments · Fixed by #7687
Closed
1 task done

Comments

@Jasonl157
Copy link

Jasonl157 commented Jun 26, 2023

Describe the bug

Updated title attribute on the ui5 component will not be reflected in the shadow dom <div class="ui5-rating-indicator-root"...>

Isolated Example

https://codesandbox.io/s/nervous-bush-fdwxk7?file=/src/App.js

Reproduction steps

  1. Open the codesandbox link above
  2. Open the chrome devtool and inspect the rating indicator component <ui5-rating-indicator> and its shadow root <div class="ui5-rating-indicator-root"...>
  3. Click the "Update Title" button to update the title attribute of the UI5 component <ui5-rating-indicator> and you will find the title of shadow root <div class="ui5-rating-indicator-root"...> remains unchanged

Expected Behaviour

Title of shadow root <div class="ui5-rating-indicator-root"...> should be updated accordingly

Screenshots or Videos

Rating.Indicator.title.issue.mov

UI5 Web Components for React Version

1.11.0 and 1.16.0

UI5 Web Components Version

1.11.0 and 1.14.0

Browser

Chrome

Operating System

No response

Additional Context

No response

Relevant log output

No response

Declaration

  • I’m not disclosing any internal or sensitive information.
@Lukas742
Copy link
Collaborator

Lukas742 commented Jun 28, 2023

Hi @Jasonl157

thanks for creating a reproducible example, I can confirm now that the internal title doesn't update. Since the RatingIndicator is developed in the UI5 Web Components team, I will forward the issue to their repo.


Hi colleagues,

even though the title behavior is not documented, it should be possible to update the tooltip of the RatingIndicator. Also, since there is no tooltip prop (like e.g. the Button provides) and custom logic for forwarding the title to the most outer element inside the shadow root is implemented for the RatingIndicator, there is no other way to update the tooltip except via title. (other than hooking into the shadow root).

--> codeSandbox using ui5wc with plain js

@Lukas742 Lukas742 transferred this issue from SAP/ui5-webcomponents-react Jun 28, 2023
@kgogov
Copy link
Member

kgogov commented Jun 29, 2023

Hello @SAP/ui5-webcomponents-topic-rl,

After looking carefully at the reported case I also agree with was said from @Lukas742. From developer perspective we should be able to update the title attribute value. I am forwarding this to you for better examination.

His code example demonstrates the issue.

Best regards,
Konstantin

@Okiana Okiana self-assigned this Jul 3, 2023
@Okiana
Copy link
Contributor

Okiana commented Jul 3, 2023

Hello @Jasonl157 ,

In the API of the ui5-rating-indicator there is no property title, therefore my suggestion could be to also update the pointed shadow dom div directly as you could access the ui5-rating-indicator component and get the dom ref - ui5-rating-indicator.getDomRef().setAttribute("title", "Updated title");

Best regards,
Evdokia

@Okiana Okiana closed this as completed Jul 3, 2023
@Lukas742
Copy link
Collaborator

Lukas742 commented Jul 3, 2023

Hi @Okiana

from my understanding it should never be necessary to hook into the shadow DOM to update the web component. That's why it's encapsulated in the first place, isn't it?
So in my opinion showing a custom title should either not be supported at all, or correctly updated as this is not the default behavior of that attribute which is available on every html element. (Here's the implementation that is forwarding the title)

Furthermore the sap.m.RatingIndicator offers an option to update the tooltip as well:

https://sapui5.hana.ondemand.com/#/entity/sap.m.RatingIndicator/sample/sap.m.sample.RatingIndicator
--> sap.ui.getCore().getElementById(<RatingIndicatorElement>.id).setTooltip("updated tooltip") results in:
image

@ndeshev
Copy link
Contributor

ndeshev commented Jul 3, 2023

Hello @Lukas742,

The title attribute is inherited from the HTMLElement itself and as you know all ui5-webcomponents are such.

In UI5 classic there is a tooltip aggregation available, thus the control supports it.

As we do not currently provide a title or tooltip property in RatingIndicator, the component has no way to know if it is updated as it is not part of its state and could not get re-rendered. If you want such API added a feature request should be created, discussed and planned.

Kind regards
Nikolay.

@Lukas742
Copy link
Collaborator

Lukas742 commented Jul 3, 2023

Hi @ndeshev

thanks for the reply. I understand your point, but when looking from a developer point of view, who is used to how the title attribute works, it's strange that it's affecting the internal element of the web component.

For example:

The Label component doesn't offer a tooltip prop as internally there is no special customization necessary for the tooltip (the component is not abstract, it doesn't offer a internal tooltip, etc.), so developers can just use title.

For other components like the Switch there is a tooltip property available, because internally the title is set. In these cases the title shouldn't be used as it won't work at all or can have strange behavior (e.g. no forwarding to icons).

Here you can find a codeSandbox with examples of what I tried to describe above.

In short:
My concerns are, that the special behavior of the title (return this.getAttribute("title") || this.defaultTooltip;) could be confusing for developers, as you can set a title but never update it (which is not the default behavior of the title attribute).

@ndeshev
Copy link
Contributor

ndeshev commented Jul 3, 2023

I also do understand your stand point. I will tag the issue as a feature request and reopen it to implement the additional API in order to avoid the current half-working tooltip behavior.

@ndeshev ndeshev reopened this Jul 3, 2023
@ndeshev ndeshev added feature request and removed bug This issue is a bug in the code labels Jul 3, 2023
@hristop hristop self-assigned this Aug 31, 2023
@hristop
Copy link
Contributor

hristop commented Sep 1, 2023

Internal BLI was created: BGSOFUIRILA-3688

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment