Skip to content
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(ui5-select): introduce value property #7865

Merged
merged 13 commits into from
Dec 1, 2023
Merged

feat(ui5-select): introduce value property #7865

merged 13 commits into from
Dec 1, 2023

Conversation

ilhan007
Copy link
Member

@ilhan007 ilhan007 commented Nov 16, 2023

Background

The topic how the form components work inside forms from the perspective of Angular and React wrappers was recently brought to us. Once more we discussed the native Select HTMLElement and the presence of "value" and that in general form components are supposed to support "value". We also agreed that it makes easier to get the resulting select's value without looping over the options and finding the selected one. Also it looks more consistent with the other form components.
Property "value" was already requested in the past but rejected - we did not add "value" because we did not want to have a second API to define the selected option as it might lead to issues as they may contradict each other.

Decision

To add "value" only if the true state remains within the options and "value" becomes kind of a shorthand for reading/updating the options, and support "value" only as a property, not as an attribute.

Changes in the PR

Introduces public value property - working only as a property, not as an attribute

  • as getter - returns the "value" or the text content of the selected option
  • as a setter - sets the selected option if the given value matches an option.
/**
* Defines the value of the component:
* <br>
* - when get - returns the value of the component, e.g. the <code>value</code> property of the selected option or its text content.
* <br>
* - when set - selects the option with matching <code>value</code> property or text content.
* <br><br>
* <b>Note:</b> If the given value does not match any existing option,
* the first option will get selected.
*
* @public
* @type { string }
* @defaultvalue ""
* @name sap.ui.webc.main.Select.prototype.value
* @since 1.20.0
* @formProperty
* @formEvents change liveChange
*/
set value(newValue: string) {
	this.selectOptions.forEach(option => {
		option.selected = !!((option.value || option.textContent) === newValue);
	});
}

get value(): string {
	return this.selectedOption?.value || this.selectedOption?.textContent || "";
}

@ilhan007 ilhan007 changed the title [draft]feat(ui5-select): support of "value" property [draft]feat(ui5-select): support of value property Nov 16, 2023
@ilhan007 ilhan007 changed the title [draft]feat(ui5-select): support of value property feat(ui5-select): support of value property Nov 18, 2023
@ilhan007 ilhan007 closed this Nov 18, 2023
@ilhan007 ilhan007 reopened this Nov 18, 2023
@ilhan007
Copy link
Member Author

FYI @dobrinyonkov

@ilhan007 ilhan007 changed the title feat(ui5-select): support of value property feat(ui5-select): introduce value property Nov 27, 2023
@ilhan007 ilhan007 merged commit 8c62d34 into main Dec 1, 2023
7 checks passed
@ilhan007 ilhan007 deleted the feat-select-value branch December 1, 2023 08:30
@ilhan007 ilhan007 added this to the 1.20.0 milestone Dec 4, 2023
ilhan007 added a commit to SAP/ui5-webcomponents-ngx that referenced this pull request Dec 5, 2023
- Select web component has new property "value" that should fix FormControl issues as the Select ngx wrapper relies on setting/getting value
SAP/ui5-webcomponents#7865

- TextArea and StepInput now provide @formEvents, @formProperty so that respective ngx wrappers are extending the GenericControlValueAccessor and thus support FormControl properly
SAP/ui5-webcomponents#7859
PetyaMarkovaBogdanova pushed a commit that referenced this pull request Dec 5, 2023
**Background**
The topic how the form components work inside forms from the perspective of Angular and React wrappers was recently brought to us. Once more we discussed the native Select HTMLElement and the presence of "value" and that in general form components are supposed to support "value". We also agreed that it makes easier to get the resulting select's value without looping over the options and finding the selected one. Also it looks more consistent with the other form components.
Property "value" was already requested in the past but rejected - we did not add "value" because we did not want to have a second API to define the selected option as it might lead to issues as they may contradict each other.

**Decision**
To add "value" only if the true state remains within the options and "value" becomes kind of a shorthand for reading/updating the options, and support "value" only as a property, not as an attribute.

**Changes in the PR** 
 Introduces public `value` property - working only as a property, not as an attribute
- as getter -  returns the "value" or the text content of the selected option 
- as a setter - sets the selected option if the given value matches an option.
ilhan007 added a commit to SAP/ui5-webcomponents-ngx that referenced this pull request Dec 6, 2023
* feat: update to ui5-webcomponents 1.20.0

- Select web component has new property "value" that should fix FormControl issues as the Select ngx wrapper relies on setting/getting value
SAP/ui5-webcomponents#7865

- TextArea and StepInput now provide @formEvents, @formProperty so that respective ngx wrappers are extending the GenericControlValueAccessor and thus support FormControl properly
SAP/ui5-webcomponents#7859

* fix: fixed generator and parser to use some of the types directly (#104)

* fix: fixed component parsing and generation

* fix: using only public methods

* fix: updated snapshots

* chore: updated snapshots

---------

Co-authored-by: Giorgi Cheishvili <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants