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

fix: force refreshValue to update input value on the client-side #3712

Merged

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Sep 13, 2022

Description

The earlier fix by @sissbruecker doesn't cover the case when ComboBox is cleared in the same round-trip as a custom value is set, e.g when calling comboBox.clear() in a customValueSet listener.

The issue manifests when trying to set a custom value on a combo-box without value. In that case, the internal customValueSet listener does have effect, setting _inputElementValue from an empty value to the custom value. However, since ComboBox is immediately cleared, it comes out like nothing has changed for the server in the end: _inputElementValue remains empty as it was, although it is not empty on the client-side.

This PR changes refreshValue() to force _inputElementValue client-side update by using the executeJs API.

Depends on

Fixes #1090

Type of change

  • Bugfix

Checklist

  • I have read the contribution guide: https://vaadin.com/docs-beta/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

@vursen vursen force-pushed the fix/combo-box/force-input-element-value-property-to-update branch from adb5471 to f4b174c Compare September 14, 2022 11:09
@@ -325,7 +329,7 @@ protected void refreshValue() {
getDataGenerator().generateData(value, json);
getElement().setPropertyJson(PROP_SELECTED_ITEM, json);
getElement().setProperty(PROP_VALUE, keyMapper.key(value));
getElement().setProperty(PROP_INPUT_ELEMENT_VALUE,
getElement().executeJs("this._inputElementValue = $0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I added this change more for the sake of consistency. It doesn't matter for the fix.

@vursen vursen marked this pull request as ready for review September 14, 2022 11:31
@vursen vursen changed the title fix: force refreshValue to update input value on the client-side (WIP) fix: force refreshValue to update input value on the client-side Sep 14, 2022
@vursen vursen requested a review from tomivirkki September 14, 2022 11:42
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell D 36 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@vursen vursen merged commit 0d90f58 into master Sep 16, 2022
@vursen vursen deleted the fix/combo-box/force-input-element-value-property-to-update branch September 16, 2022 08:30
@vaadin-bot
Copy link
Collaborator

Hi @vursen and @vursen, when i performed cherry-pick to this commit to 23.2, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 0d90f58
error: could not apply 0d90f58... fix: force refreshValue to update input value on the client-side (#3712)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Collaborator

Hi @vursen and @vursen, when i performed cherry-pick to this commit to 23.1, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 0d90f58
error: could not apply 0d90f58... fix: force refreshValue to update input value on the client-side (#3712)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Collaborator

Hi @vursen and @vursen, when i performed cherry-pick to this commit to 14.8, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 0d90f58
error: could not apply 0d90f58... fix: force refreshValue to update input value on the client-side (#3712)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.3.0.alpha1 and is also targeting the upcoming stable 23.3.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ComboBox.clear() has no effect when custom values are enabled and a custom value is entered
3 participants