-
Notifications
You must be signed in to change notification settings - Fork 281
feat(ui5-popover): implement resizable popover #12623
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
base: main
Are you sure you want to change the base?
Conversation
|
🚀 Deployed on https://pr-12623--ui5-webcomponents-preview.netlify.app |
alexandar-mitsev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- during the resize, the arrow constantly flickers. minor issue, but quite visible.
- sometimes the resize arrow is on the wrong side. This happens if the opener is longer than the popover - e.g. in a very long date picker input, the popover is in leftmost position and the arrow is also in the left bottom arrow. To reproduce set resizable to true on the datepicker input (second input) on test/pages/Popover.html. Same is reproducible in the "Horizontal Align" test section in Popover.html
- during resize - if you do it quick, the resizing starts to lag. Can stay like that, but if possible to fix, will be better
| } | ||
|
|
||
| if (isClickInRect(event, popup.getBoundingClientRect())) { | ||
| if ((popup as Popover).isClicked(event)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it could be cleaner to have method isClicked in the Popup itself then override it in the Popover
this way, when someone adds a new type of Popup, this code will still work
or add a check in the beginning of the funcion that this is a popover
generally casting to Popover without being sure it is a popover could bring unexpected errors at runtime
| const isRtl = this.isRtl; | ||
|
|
||
| const opener = this.getOpenerHTMLElement(this.opener); | ||
| const openerRect = opener!.getBoundingClientRect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sometimes throws error "TypeError: Cannot read properties of undefined (reading 'getBoundingClientRect')"
to reproduce - have a popover which has no opener, and opener is assigned only just before open. Then an error is thrown
I reproduced it by setting resizable to true and tested on /packages/main/test/pages/Popover.html
This leaves a number of popover opened and visible before clicking the button on the test page.
JIRA: BGSOFUIRODOPI-3384
Certain styles for slotted elements in the footer and header are overridden, similar to how it is done in the UI5 dialog.