chore(ui5-toolbar): toolbar item wrapper introduced #12243
Conversation
…nto poc-toolbar-item
…nto poc-toolbar-item
|
🧹 Preview deployment cleaned up: https://pr-12243--ui5-webcomponents.netlify.app |
|
🚀 Deployed on https://pr-12243--ui5-webcomponents-preview.netlify.app |
|
|
||
| interface IOverflowToolbarItem extends HTMLElement { | ||
| eventsToCloseOverflow?: string[] | undefined; | ||
| _selfOverflowed?: boolean | undefined; |
There was a problem hiding this comment.
rename?
overflowCloseEvents
hasOverflow
|
|
||
| // Method called by ui5-toolbar to inform about the existing toolbar wrapper | ||
| checkForWrapper() { | ||
| const tagName = this.item?.[0]?.localName as keyof typeof this.predefinedWrapperSet; |
There was a problem hiding this comment.
Tag name should be calculated from the constructor's metadata. Like we do in getClosingEvents. Perhaps let's move to a dedicated getter get itemTagName
| @@ -0,0 +1,8 @@ | |||
| /* This style we need for the element inside Popover to not fire the click event on empty space */ | |||
| :host([is-overflowed]:not([expand-in-overflow])) { | |||
There was a problem hiding this comment.
this might not be needed once we make everything 100% or since we removed the wrapper div around the slot
| */ | ||
|
|
||
| @property({ type: Boolean }) | ||
| expandInOverflow: boolean = false; |
There was a problem hiding this comment.
do we need this, shoudn't everything be 100%, what components should not be full width in popover?
| _isRendering = true; | ||
| _maxWidth = 0; | ||
| fireCloseOverflowRef = this.fireCloseOverflow.bind(this); | ||
| _kind = "ToolbarItem"; |
There was a problem hiding this comment.
We can remove this. Dedicated wrappers are not populating a slot
| </div> | ||
| ); | ||
| } | ||
| const separatorClass = item.isSeparator ? " ui5-tb-separator ui5-tb-separator-in-overflow" : ""; |
There was a problem hiding this comment.
This could be jsx dynamic class
| * @since 2.19.0 | ||
| */ | ||
| @property({ type: Boolean }) | ||
| _selfOverflowed: boolean = false; |
There was a problem hiding this comment.
This could be a getter, as it's not used in CSS, nor its ever set outside rerendering phase?
get hasOverflow() {
return this.items?.[0].hasOverflow
}| @slot({ | ||
| "default": true, type: HTMLElement, invalidateOnChildChange: true, | ||
| }) | ||
| item!: IOverflowToolbarItem[]; // here |
There was a problem hiding this comment.
remove comment // here
|
|
||
| // We want to close the overflow popover, when closing event is being executed | ||
| getClosingEvents(): string[] { | ||
| const ctor = this.getSlottedNodes<IOverflowToolbarItem>("item")[0]?.constructor as typeof ToolbarItem; |
feat(ui5-toolbar-item): Refactor to a physical item for enhanced overflow logic
The
ui5-toolbar-itemhas been refactored from an abstract item to a physical item, enabling the transfer of logic foroverflowPriorityandpreventOverflowClosingdirectly to it.Key Enhancements:
ui5-toolbar-itemcan now wrap all types of components, allowing them to participate in the overflow toolbar logic seamlessly.expandInOverflowproperty.selfOverflowedproperty.This enhancement improves flexibility and ensures better integration of components within the overflow toolbar logic.