Skip to content

fix: TextField defaultValue change #2388

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

Merged
merged 8 commits into from
Jan 1, 2023

Conversation

AmitShwarts
Copy link
Contributor

@AmitShwarts AmitShwarts commented Dec 22, 2022

Description

Currently when trying to change default value then it is not rendered.
This PR will fix this issue.

Changelog

TextField - fix defaultValue not changing

@AmitShwarts AmitShwarts marked this pull request as draft December 22, 2022 10:56
@AmitShwarts AmitShwarts marked this pull request as ready for review December 22, 2022 11:04
Copy link
Contributor

@lidord-wix lidord-wix left a comment

Choose a reason for hiding this comment

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

Can you share an example?
When I render a simple example of TextField and update the defaultValue prop it does the update as I expected..

@AmitShwarts
Copy link
Contributor Author

Can you share an example? When I render a simple example of TextField and update the defaultValue prop it does the update as I expected..

Sorry, forgot to mention that it happens on web.
i updated the webDemo to show this use case.

Copy link
Contributor

@lidord-wix lidord-wix left a comment

Choose a reason for hiding this comment

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

I updated the changes from this PR in the node modules and I can't see any change when pressing the 'update default value' button..
Did you check it?

renderTree.getByDisplayValue('someValue');
});

it('on web should reset defaultValue when prop changed after first render', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest adding the same test without setting Constants.isWeb to true since the same behavior should also apply on mobile

@lidord-wix lidord-wix merged commit 11fad95 into master Jan 1, 2023
@AmitShwarts AmitShwarts deleted the fix/text-field-default-value-change branch January 1, 2023 08:50
@lidord-wix lidord-wix mentioned this pull request Jan 19, 2023
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