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.

Sign up for GitHub

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

Jump to bottom

fix(input-mixin): reset _hasInputValue on programmatic value clear #5599

Merged
merged 5 commits into from
Feb 27, 2023
Merged

fix(input-mixin): reset _hasInputValue on programmatic value clear #5599

merged 5 commits into from
Feb 27, 2023

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Feb 24, 2023 •
edited
Loading

Description

The PR resets _hasInputValue to false on programmatic value clear so that Flow won't treat the empty value as bad input. It also mentions in the JSDoc that _hasInputValue only indicates true if the input has been entered by the user. Ideally, given the way the property works, it should be renamed to _hasUserInput but we decided not to change that in the scope of this issue.

Note
There will be more PRs like this one. They will cover other cases where the input value can be changed programmatically.

Part of #5410

Type of change

  • Bugfix

@vursen vursen changed the title fix: reset _hasInputValue on programmatic clear fix: reset _hasInputValue on programmatic value clear Feb 24, 2023
@vursen vursen marked this pull request as ready for review February 24, 2023 13:14
web-padawan
web-padawan approved these changes Feb 24, 2023
View reviewed changes
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

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

LGTM. Added two suggestions on the test suite namings, these aren't blockers though.

packages/field-base/test/input-mixin.test.js Outdated Show resolved Hide resolved
packages/field-base/test/input-mixin.test.js Outdated Show resolved Hide resolved
vursen and others added 2 commits February 25, 2023 10:54
@vursen @web-padawan
Update packages/field-base/test/input-mixin.test.js
3694d25 
Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
@vursen @web-padawan
Update packages/field-base/test/input-mixin.test.js
2260d32 
Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
@sonarcloud
Copy link

sonarcloud bot commented Feb 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@web-padawan web-padawan removed the request for review from bwajtr February 27, 2023 12:11
@web-padawan web-padawan merged commit 85759d6 into main Feb 27, 2023
@web-padawan web-padawan deleted the fix/reset-has-input-value-on-clear branch February 27, 2023 12:11
vaadin-bot pushed a commit that referenced this pull request Feb 27, 2023
@vursen @vaadin-bot
fix: reset _hasInputValue on programmatic value clear (#5599)
0734807
web-padawan pushed a commit that referenced this pull request Feb 27, 2023
@vaadin-bot @vursen
fix: reset _hasInputValue on programmatic value clear (#5599) (#5605)
aef7168 
Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>
@vursen vursen changed the title fix: reset _hasInputValue on programmatic value clear fix(input-mixin): reset _hasInputValue on programmatic value clear Apr 25, 2023
@vursen vursen mentioned this pull request Apr 25, 2023
Closed
12 tasks
@mukherjeesudebi mukherjeesudebi mentioned this pull request Apr 27, 2023
Merged
@mukherjeesudebi mukherjeesudebi mentioned this pull request May 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@web-padawan web-padawan web-padawan approved these changes

@ugur-vaadin ugur-vaadin ugur-vaadin approved these changes

Assignees
No one assigned
Labels
cherry-picked-24.0 target/24.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vursen @web-padawan @ugur-vaadin @vaadin-bot