-
Notifications
You must be signed in to change notification settings - Fork 273
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
feat: add property initializers #8846
Conversation
packages/main/src/ComboBox.ts
Outdated
|
||
@property({ type: Object, noAttribute: true, multiple: true }) | ||
@property({ type: Array, noAttribute: true }) | ||
_filteredItems!: Array<IComboBoxItem>; |
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.
i think that we have to initialise with empty array here
SAP Belize theme was deprecated as of version 1.22.0. With this change, we are performing cleanup for the deprecated theme. BREAKING CHANGE: Remove SAP Belize theme Related to #8461
Rename `changeWithClick` to `withScroll` in the `WizardStepChangeEventDetail`. If the application code relied on `changeWithClic` event detail, it should now rely on `withScroll` event detail instead. The `withScroll` parameter will be set to true when the event is fired due to user scrolling. BREAKING CHANGE: `changeWithClick` was renamed to `withScroll` in the `WizardStepChangeEventDetail`. JIRA: BGSOFUIRILA-3867 Related to: #8461
…independently from button (#8669) Remove the inheritance of `ToggleButton` in `SegmentedButtonItem` control. Change the `pressed` property to `selected`. BREAKING CHANGE: The `ui5-segmentedbutton-item` `pressed` property is called `selected` now. Previously the application developers could use the ui5-segmentedbutton-item as follows: ```html <ui5-segmented-button> <ui5-segmented-button-item pressed> Option 1</ui5-segmented-button-item> <ui5-segmented-button-item>Option 2</ui5-segmented-button-item> <ui5-segmented-button-item>Option 3</ui5-segmented-button-item> </ui5-segmented-button> ``` Now the application developers should use the ui5-segmentedbutton-item as follows: ```html <ui5-segmented-button> <ui5-segmented-button-item selected> Option 1</ui5-segmented-button-item> <ui5-segmented-button-item>Option 2</ui5-segmented-button-item> <ui5-segmented-button-item>Option 3</ui5-segmented-button-item> </ui5-segmented-button> ``` Related to: #8461
Users now can open `ui5-toast` using the `open` property. A new event `after-close` is introduced. It may be used to sync app state with the `open` state of the `ui5-toast`. BREAKING CHANGE: The Toast#show method has been replaced by `open` property. If you previously used `toast.show()` to show the toast, you must now se `toast.open=true`. Related to: #8461
@@ -210,29 +209,29 @@ class Table extends UI5Element { | |||
/** | |||
* Defines the accessible ARIA name of the component. | |||
* | |||
* @default "" | |||
* @default undefined |
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.
what is the motivation of this change? isn't it now different than the way how DOM API works?
if an element has no id property/attribute what returns accessing the $0.id? it does not return undefined
but empty string since it is defined as DOMString in the IDL. I would expect the same. Even setting undefined back $0.id = undefined
should convert the undefined
to 'undefined'
it should not allow setting undefined
back.
Same is true for enums for example for the input, setting type to e.g. $0.type="petar"
returns back as text
when $0.type
is accessed again although the type attribute will be written as type="petar"
. So there is a validation for enums and silently fails to default value.
Also true for numeric values. for example for input type number, setting an invalid value $0.valueAsNumber = 'petar'
will log an error and return NaN
when $0.valueAsNumber
is read back again.
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.
a lot of topics to cover here, posting as separate comments
undefined
is a valid value for a lot of properties (there are properties that acceptundefined
and are initialized withundefined
even before the change). accessibleName specifically is used in the AriaLabelHelper.ts and has a truthy check that treats it as undefined even when empty string is passed.
if (accessibleEl.accessibleName) {
return accessibleEl.accessibleName;
}
return undefined;
There are a lot of acc places where an empty string is different from undefined and a lot of checks for "" that return undefined
(empty string is unwanted from acc perspective, but used to come from the framework).
If you still want accessible name to be initialized to "", feel free to change it. This will also remove the ?
and all typescript users will be unable to assign undefined
. It's up to you, but my guidance is, if undefined
and ""
are treated the same way, then simply go with undefined
as default.
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.
- reading properties and expecting some fallback to be applied - reading properties is used in jquery style programming, real programming models like react/angular/vue will have some kind of model and will only write to the properties, or read values from events.
In this case, we didn't want to mimic existing browser behaviour if it means adding more code, especially code that introduces "magic". The typescript type already specifies valid values and components expect to work with valid values anyway.
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.
- enums and fallback - assigning an invalid value is clearly a developer error and should be fixed. If the developer assigned
button.design = "Netigave"
(notice the typo), it would be better to find out and correct it instead of falling back to "Default"
Enums values when used in CSS via attribute selectors already have a fallback - there should be a css rule without the attribute that will catch invalid values as well.
EDIT: enums value are also hardcoded in templates by the developers, they are not bound to some model, nor are they determined from user input. If this is the case, the application should take care to privide a default, not the framework.
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.
- same for numbers - if someone passes a string to a number field, they clearly made an error that should be fixed instead of falling back to some value and masking the error. The component API clearly states that a number is expected, why should it work any other way?
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.
- in the future, ES standard decorators will provide means to differentiate what the initializer value was. Then we could use it as a fallback as well. As it is now with experimental decorators, there is no way to find out what the initializer value was - it is indistinguishable from normal setters. So what you describe could be introduced in the future.
Another option is to create a decorator (fallback, or guard) that instruments the setter and takes an addtional value, but this will be repeating the values like this which is undesired:
@property()
@fallback("text")
type = "text"
This repetition does not look good, and normal cases should be covered when using typescript with a programming model - it simply isn't necessary to have fallbacks.
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 have a call if you want a deeper discussion or use one of the regular meetings.
With the introduction of property initializers (#8846) many properties have changed they default values or types but on some places: - JSDoc wasn't changed - Properties with declared `declare` keyword were using wrong type
With the introduction of property initializers (#8846) many properties have changed they default values or types but on some places: - JSDoc wasn't changed - Properties with declared `declare` keyword were using wrong type This change fixes most of the things until custom element analyzer plugin is adjusted to generate correct output
With the introduction of property initializers (#8846) many properties have changed they default values or types but on some places: - JSDoc wasn't changed - Properties with declared `declare` keyword were using wrong type This change fixes most of the things until custom element analyzer plugin is adjusted to generate correct output
Initial values for properties
Since a web component can be used in plain HTML with just a simple tag
<ui5-button></ui5-button>
, all properties are optional. Providing initial (default) values for the properties used to be part of the metadata object (later also in the decorator) with adefaultValue
field.This actually has two mixed usages:
With this change, component development is switching to the standard way of using property initializers
Initial values
Initial values are no longer magically provided by the framework. This means that properties should be either optional or initialized, and very rarely (for complex objects) described as non-null
Fallback values
All runtime checks for properties (especially enumerations) are removed. All typechecking is left to the TypeScript compiler and assigning an invalid value to an enumeration or a number/boolean field is considered a bug that should be fixed instead of the framework silently masking it by providing a fallback value.
This means that enums are no longer supported for the
type
field in the decorator:Migrating old code
enums
boolean
numbers
type: Number
strings
DOMReference
type
field in decoratorsThe type field is now used only for attribute conversion. There are the following types
type: Boolean
treats the converstion to/from attribute as boolean
type: Number
treats the converstion to/from attribute as number
type: Array
no special handling, same as
noAttribute: true
type: Object
no special handling, same as
noAttribute: true
Dev time checks
Passing
Number
orBoolean
values to attributes that are not annotated with the same type will cause a console error and fail the tests, mostly because passing such values means the type annotation was forgotten.This means the test pages have to pass strings for fields where a string is expected (similar to how typescript will not allow passing a number to a string property)