[Toolbar(V2)] ToolbarButton with Modals Menu rendered in the wrong place
Describe the bug
Adding a Menu to a ToolbarButton using state or the Modals API (https://sap.github.io/ui5-webcomponents-react/v2/?path=/docs/user-feedback-modals--docs) doesnt render the menu in the right place.
The above code sample is basically just copied from the doc
import {
MenuItem,
Modals,
Toolbar,
ToolbarButton,
} from '@ui5/webcomponents-react';
...
<Toolbar>
<ToolbarButton
id="modals-show-menu"
text={'Show menu'}
onClick={() => {
Modals.showMenu({
opener: 'modals-show-menu',
headerText: 'Menu Title',
children: (
<>
<MenuItem text={'Click me'} onClick={() => {}} />
</>
),
});
}}
/>
</Toolbar>
There are several things which might be unrelated that I have also noticed: The behaviour doesnt change where we place the <Modals /> element Toasts are showing a similar behaviour in some cases, dependent on where in the tree the <Modals /> element is placed The menu is displayed properly using @ui5/webcomponents-react-compat Toolbar (Legacy) with a standard button We are having similar issues in the latest 1.x versions of webcomponents(-react)
Isolated Example
https://stackblitz.com/edit/github-f9pvpx
Reproduction steps
No response
Expected Behaviour
The Menu is displayed attached to the ToolbarButton
Screenshots or Videos
You can see the menu attached at the wrong place in the bottom left
UI5 Web Components for React Version
2.2.0
UI5 Web Components Version
2.3.0
Browser
Chrome
Operating System
Windows 11
Additional Context
No response
Relevant log output
No response
Organization
No response
Declaration
- [X] I’m not disclosing any internal or sensitive information.
Hi @gitgdako
you're using the same id for multiple elements, this is the reason why it's not working for the button outside of the toolbar opening a Menu.
The ToolbarButton itself is an abstract (docs) ui5 web component and therefore the id is not forwarded to the actual element inside the shadow root.
Usually you should be able using the getDomRef method of the ui5-toolbar-button to get the reference for the actual element, but it doesn't seem to work for the ToolbarButton. I've created an issue for this: https://github.com/SAP/ui5-webcomponents/issues/9996
Because of this, it's pretty complicated getting a reference to the actual button where the popover should be opened:
https://stackblitz.com/edit/github-f9pvpx-m5rxp2?file=src%2FApp.tsx,package.json
Since the Toolbar and all of its subcomponents are developed by the ui5-webcomponents team, I'll forward this issue to their repo.
The menu is displayed properly using @ui5/webcomponents-react-compat Toolbar (Legacy) with a standard button We are having similar issues in the latest 1.x versions of webcomponents(-react)
Please create a dedicated issue for this, since the Toolbar v1 is a completly different component, maintained in the ui5-webcomponents-react repo.
Hi Colleagues,
could you please check what the intended way of opening a popover via a ui5-toolbar-button should be? Currently it's only possible by entering the shadowRoot and referencing the respective internal button as opener. in the example above the click handler should mainly be the same as with plain js. If you still need an example without our wrapper or React please let me know.
For the ShellBarItem there was a targetRef event detail introduced for the same reason, maybe this is something to consider here as well.
type ShellBarItemClickEventDetail = {
targetRef: HTMLElement,
};
Sorry for the faulty Code sample. Thanks for forwarding to the relevant people. As for the mention of 1.x: I meant the ToolbarV2 in 1.x, which I assume faces the same issue. For us it does not need to be fixed there as we will move to 2.x
Hello @ui5-webcomponents-topic-p,
It seems the menu is not opened because of undefined getDomRef() returned by ui5-toolbar-button, alse see the related issue #9996
Best Regards, Nikolay Hristov
Hi @kgogov @Lukas742, when will this be released? I assumed it would be in 2.4 and tested it and have some new findings that can be seen in the adapted Sample https://stackblitz.com/edit/github-f9pvpx The menu is now rendered in the top left. Am I using the opener reference wrong? The compat toolbar now does the same intially, but the menu then jumps to the opener
Hi @gitgdako
you can't set an id there - please see my answer above about abstract elements. Also, the StackBlitz example isn't updated to 2.4, so the fix isn't available there.
I've updated your example and used the internal ui5-button element (via getDomRef) as opener and now it's working as intended.
https://stackblitz.com/edit/github-f9pvpx-ynsqxn?file=src%2FApp.tsx,package.json
Hi @kgogov
I’d argue that the issue isn’t resolved by the linked PR. The PR only addresses the problem with getDomRef (corresponding issue), it doesn’t provide a way to obtain the correct element reference via the click event (similar to the click event of the ShellBar) or outlines the recommended way to open a popover in the docs. This could lead to confusion for other developers, as I think that getDomRef isn’t widely recognized, especially for abstract components.
The documentation of the modals https://sap.github.io/ui5-webcomponents-react/v2/?path=/docs/user-feedback-modals--docs doesnt really make it clear that in this specific case the usage is different (and also not at all intuitive). This need to be addressed by updating the doc accordingly or having a consistent usage (which would obviosuly be preferrable)
Hi @gitgdako this behavior occurs with or without using our Modals API. Abstract components are a general concept, which is why we can't outline every situation where they might be relevant. We describe this particularity in our FAQ. If you feel like something is missing there, please let us know.
This might be just my gut feeling, but using a Menu on a button (which is "standard" functionality covered in your storybook) within a toolbar isnt such an edge case. I dont think without raising an issue or stumbling across this one I would never be able to get this to work. The alternative solution would be a component explicitly meant for usage in a Toolbar [0]
[0] https://github.com/SAP/ui5-webcomponents/issues/9851
@Lukas742 There is an other issue with the given solution: When clicking the button the menu pops up in the top left corner before it snaps to the opener. So it is set to open in its placeholder state before being moved. This isnt a major issue but not really beautiful Reproduce: Your updated stackblitz
- https://stackblitz.com/edit/github-f9pvpx-ynsqxn?file=src%2FApp.tsx,package.json
- Press the button to see menu pop up in the top left corner (can be done multiple times)
Hi @kgogov
the behavior described by @gitgdako occurs when the Menu is conditionally rendered. You can find an example using only ui5wc without our wrapper or React here: https://stackblitz.com/edit/github-8upxfb?file=index.html,main.js
Should this be discussed in this GitHub issue, or would it be better to create a new one?
Hi @Lukas742,
Thank you for providing the example and for outlining the behavior when the Menu is conditionally rendered.
To keep things organized and clear, I suggest creating a new GitHub issue to discuss this specific behavior separately.
As I see it, there are 3 issues at hand here:
- the original issue of the component not working at all, which is "fixed" imo, although not perfectly
- documentation and reproducability: As stated above this isnt intuitive at all and can hardly be recreated by a dev. Either document it well in your storybook or create a different way of using a menu with a toolbar button)
- the menu poping up in the corner: that can be moved to a separate issue. Please do create it yourself because you can probably attach more relevant information
Hello! I’d like to share a quick update on the progress:
We’ve been working on enabling popovers to open through abstract elements, such as ui5-toolbar-button.
You can find the details here: https://github.com/SAP/ui5-webcomponents/pull/10140
Hello,
Based on the latest changes, I believe we can close this ticket because:
- With the recent update, it is now possible to use abstract components, such as
ui5-toolbar-button, as anopener. - This change also allows calling
getDomRef()on specific abstract elements within theToolbar. - Lastly, we added the
targetRefproperty to theevent.detailsobject, which can now be used to open aPopover.
If there are still issues related to the Toolbar component, please let us know.
Thank you to @Lukas742 for the detailed analysis and to @wvudako for reporting the issue.