SAP / ui5-webcomponents

UI5 Web Components - the enterprise-flavored sugar on top of native APIs! Build SAP Fiori user interfaces with the technology of your choice.
https://sap.github.io/ui5-webcomponents/
Apache License 2.0
1.54k stars 265 forks source link

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

Closed Jasonl157 closed 1 year ago

Jasonl157 commented 1 year ago

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

https://github.com/SAP/ui5-webcomponents-react/assets/131832797/8ed6abff-e8ba-49f4-a0be-1f08acd81377

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

Lukas742 commented 1 year ago

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

kgogov commented 1 year ago

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 commented 1 year ago

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

Lukas742 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

hristop commented 1 year ago

Internal BLI was created: BGSOFUIRILA-3688