feat(text-field): reset value on type change #6834
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43081j
requested review from
EisenbergEffect,
chrisdholt and
nicholasrice
as code owners
43081j
force-pushed
the
input-type-values
branch
from
7cbdbd8 to
360b511
Compare
43081j
force-pushed
the
input-type-values
branch
from
360b511 to
bf31e52
Compare
scomea
approved these changes
Jan 17, 2024
change/@microsoft-fast-foundation-fbee44f7-f673-4961-a65a-be11acf4b0eb.json
Outdated
Show resolved
Hide resolved
change/@microsoft-fast-foundation-fbee44f7-f673-4961-a65a-be11acf4b0eb.json
Outdated
Show resolved
Hide resolved
scomea
requested changes
Jan 17, 2024
|
sorry only just got around to this have rebased on master and updated the change file |
43081j
force-pushed
the
input-type-values
branch
from
bf31e52 to
5a59fd6
Compare
scomea
approved these changes
Jan 23, 2024
This changes `fast-text-field` to reset its value any time `type` changes. The reason for this is that switching between two incompatible input types may reset the inner control's value, which won't otherwise be reflected to the outer element. For example, using `type="text"` and typing a value into the input, then changing to `type="number". This should reset the value, since the inner control will have already done so.
43081j
force-pushed
the
input-type-values
branch
from
5a59fd6 to
54e4a09
Compare
|
have rebased onto latest master any chance anyone can take a look at merging this some day? 👀 |
bheston
approved these changes
Mar 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes
fast-text-fieldto reset its value any timetypechanges.The reason for this is that switching between two incompatible input types may reset the inner control's value, which won't otherwise be reflected to the outer element.
For example, using
type="text"and typing a value into the input, then changing totype="number". This should reset the value, since the inner control will have already done so.🎫 Issues
Fixes #6827
👩💻 Reviewer Notes
In material, this is solved by resetting the value any time the component's properties change at all. Maybe we should do the same here? I just wasn't sure if there's a lifecycle callback in FAST for when any properties change.
I'm also not too sure the test I've added is consistent with how you usually test this stuff. I didn't see any other tests testing the members of the element itself, only via attributes.
If you want to solve this another way, let me know and I'm happy to update.
📑 Test Plan
Added a new test for changing
typeafter input has occurred.✅ Checklist
General
$ yarn changeComponent-specific