Skip to content
This repository was archived by the owner on Feb 13, 2026. It is now read-only.

ui v2: text field#701

Merged
danielhochman merged 6 commits intoUIV2from
uiv2-text-field
Dec 1, 2020
Merged

ui v2: text field#701
danielhochman merged 6 commits intoUIV2from
uiv2-text-field

Conversation

@danielhochman
Copy link
Collaborator

Description

Implement new text field design with the exception of the "clear" control, which requires some more though and potentially refactoring to standardize on controlled vs uncontrolled components.

Testing Performed

See stories.

Co-authored-by: Derek <dschaller@users.noreply.github.com>
Comment on lines +100 to +102
| "onChange"
| "onFocus"
| "onKeyDown"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth having both onChange and onKeyDown? I'm trying to think of a use case where someone would want to specify two different handlers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you're using onKeyDown in the resolver so i kept it, wasn't sure of implication of changing it and didn't want to change interfaces beyond style props

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 we should figure out a single callback but it can be done in a separate PR

Copy link
Contributor

@dschaller dschaller left a comment

Choose a reason for hiding this comment

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

pending removing commented out code

Co-authored-by: Derek <dschaller@users.noreply.github.com>
@danielhochman danielhochman merged commit 58a6aae into UIV2 Dec 1, 2020
@danielhochman danielhochman deleted the uiv2-text-field branch December 1, 2020 20:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants