-
Notifications
You must be signed in to change notification settings - Fork 279
chore(ui5-toolbar-select): value and label added #11972
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?
Changes from all commits
04fa785
0f2decc
bed5075
2788558
e7c64c1
171af8b
8a4aa5f
a5565df
cf5b455
b6bd247
f02f390
967dbc0
577e0f5
f2b41a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import slot from "@ui5/webcomponents-base/dist/decorators/slot.js"; | |
import event from "@ui5/webcomponents-base/dist/decorators/event-strict.js"; | ||
import type ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js"; | ||
import ToolbarSelectCss from "./generated/themes/ToolbarSelect.css.js"; | ||
import type Select from "./Select.js"; | ||
|
||
// Templates | ||
import ToolbarSelectTemplate from "./ToolbarSelectTemplate.js"; | ||
|
@@ -87,9 +88,22 @@ class ToolbarSelect extends ToolbarItem { | |
* **Note:** Use the `ui5-toolbar-select-option` component to define the desired options. | ||
* @public | ||
*/ | ||
@slot({ "default": true, type: HTMLElement, invalidateOnChildChange: true }) | ||
@slot({ | ||
"default": true, | ||
type: HTMLElement, | ||
invalidateOnChildChange: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need invalidateOnChildChange here? |
||
}) | ||
options!: Array<ToolbarSelectOption>; | ||
|
||
/** | ||
* Defines the HTML element that will be displayed in the component input part, | ||
* representing the selected option. | ||
* @public | ||
* @since 2.13.0 | ||
*/ | ||
@slot() | ||
label!: Array<HTMLElement>; | ||
|
||
/** | ||
* Defines the value state of the component. | ||
* @default "None" | ||
|
@@ -124,6 +138,34 @@ class ToolbarSelect extends ToolbarItem { | |
@property() | ||
accessibleNameRef?: string; | ||
|
||
/** | ||
* Defines the value of the component: | ||
* | ||
* @public | ||
* @default "" | ||
* @since 2.13.0 | ||
*/ | ||
@property() | ||
set value(newValue: string) { | ||
if (this.select && this.select.value !== newValue) { | ||
const selectedOption = this.select.options.find(option => option.textContent === newValue); | ||
this.select.value = newValue; | ||
selectedOption && this.fireDecoratorEvent("change", { targetRef: this.select, selectedOption }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As of now the select isn't firing the change event if the value changes. In future, if the selects starts firing it we have a handler below that will catch and re-fire it from the toolbar select. If this is a requirement now, we can add it to the select itself. |
||
} | ||
this._value = newValue; | ||
} | ||
|
||
get value(): string | undefined { | ||
return this.select ? this.select.value : this._value; | ||
} | ||
|
||
get select(): Select | null { | ||
return this.shadowRoot!.querySelector<Select>("[ui5-select]"); | ||
} | ||
|
||
// Internal value storage, in case the composite select is not rendered on the the assignment happens | ||
_value: string = ""; | ||
|
||
onClick(e: Event): void { | ||
e.stopImmediatePropagation(); | ||
const prevented = !this.fireDecoratorEvent("click", { targetRef: e.target as HTMLElement }); | ||
|
@@ -174,6 +216,10 @@ class ToolbarSelect extends ToolbarItem { | |
width: this.width, | ||
}; | ||
} | ||
|
||
get hasCustomLabel() { | ||
return !!this.label.length; | ||
} | ||
} | ||
|
||
ToolbarSelect.define(); | ||
|
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.
Let's move this test to a ToolbarSelect.cy.tsx file and test the value and slot members separately.
Also add few more tests for the value property, for example changing the value should change the selection or passing a value that has no corresponding option, etc.
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.
Lets also test with the ui5-toolbar-select-option component