ui5-webcomponents icon indicating copy to clipboard operation
ui5-webcomponents copied to clipboard

[Popover]: Cannot use DOM reference as the opener of the popover.

Open allen138 opened this issue 1 year ago • 3 comments

Describe the bug

The opener support either a DOM id string, or a HTML Element DOM reference. However, the type is only defined as string. Please see: react Popover/index.tsx#L94 vs ui5-webcomponents Popover.ts#L174. In my scenario I would like to use a dom ref, but the types are failing with the following: image

As a workaround I tried something like

opener={(opener.current as unknown as string)}

But then it fails later as Valid opener id is required. since the dom reference is actually getting passed as a string to the webcomponent "[Object HTMLElement]".

Isolated Example

stackblitz sample

Reproduction steps

  1. Open the isolated example, and notice the ts compilation error
  2. Then, if you hack the type safety, notice the popover still does not open and you get the warning in the console. ...

Expected Behaviour

You should be able to use a dom ref as the opener for popovers.

UI5 Web Components for React Version

1.22.2

UI5 Web Components Version

1.19.1

Browser

Chrome

Organization

SF

Declaration

  • [X] I’m not disclosing any internal or sensitive information.

allen138 avatar Apr 02 '24 20:04 allen138

Hi @allen138, I think the description of the UI5 Web Component is misleading in that case. The UI5 Web Components are also supporting only strings as prop / attribute when passing the opener prop in a declarative way. You can also use the DomReference type if you set the opener attribute via JavaScript, e.g. in an useEffect. --> I'm going to transfer the issue to the UI5 Web Components repo.


Hi colleagues, I think we have some room for improvement in the JSDocs for the Popups with an opener attribute. I would like to enhance the description to include a note, that an opener can only be passed as DomReference when using JavaScript:

Defines the ID or DOM Reference of the element that the popover is shown at. When using this attribute in a declarative way you must only use the id as string of the element you want to show the popover at. You can only set the opener attribute to a DOM Reference when using JavaScript.

If you agree with that enhancement I'm happy to open a PR for that.

MarcusNotheis avatar Apr 08 '24 09:04 MarcusNotheis

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

Could you please check the proposal for documentation enhancement?

Kind Regards, Niya

niyap avatar Apr 08 '24 11:04 niyap

Hello @MarcusNotheis ,

We agree with your proposal, please go forward with opening a PR.

Best regards, Petar

dimovpetar avatar Apr 24 '24 07:04 dimovpetar

The UI5 Web Components are also supporting only strings as prop / attribute when passing the opener prop in a declarative way. You can also use the DomReference type if you set the opener attribute via JavaScript, e.g. in an useEffect. --> I'm going to transfer the issue to the UI5 Web Components repo.

@MarcusNotheis I thought the wrapper was already doing this - when it gets an object for a property that accepts an object, it should create its own useEffect and set the property. lit-html is already doing this, you can set properties directly from the template. Perhaps in react it is more inconvenient that it is not immediate but hidden behind useEffect, but that's what I imagined one of the use cases for wrappers is to make development easier.

pskelin avatar May 15 '24 08:05 pskelin