Skip to content

The new TextField (Incubator) #854

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 48 commits into from
Aug 18, 2020
Merged

The new TextField (Incubator) #854

merged 48 commits into from
Aug 18, 2020

Conversation

ethanshar
Copy link
Collaborator

@ethanshar ethanshar commented Jul 15, 2020

Recreate the TextField component basing on these guidelines

  • Add parity with the old TextField
  • Use ThemeManager to apply our private UI
  • QA the component for missing functionality and UI issues
  • Start planning a migration process

Features

  • General structure
  • Support floating placeholder
  • Refine floating placeholder behavior (support custom typography, add scale animation)
  • Support accessory buttons
  • Support style per state (focused, invalid, disabled, blur)
  • Allow styling Label
  • Allow styling ValidationMessage
  • Connect to validation logic
  • Support checking validation in blur, change, state
  • Consider adding presets for different UI looks
  • Fix Android spacing issues, make sure UI is aligned between the two Platforms
  • Support char counter
  • Support helper text (renamed to hint)
  • Support render validation message on top/bottom
  • Support multiline
  • Verify controlled behavior
  • Support Accessibility
  • Fix ref warning
  • Fix typescript errors
  • Support theme props
  • RTL

Notes for Review

  • The reason for this reimplementation is the fact this component contains legacy code which is impossible to maintain today
  • There are lots of files to go over, no need to review generatedTypes files
  • In the new implementation I broke the component into very smaller components, each with a specific purpose.
    It should be much more readable to go over the code. And in general that's how I see we should write complex components.
  • I decided to omit specific UI/Functionality decisions that are internal to the Wix App in order to make the component more generic. We should be able to customize everything using ThemeManager.
  • There are spacing differences between Android and iOS which are impossible to unify (height of the input for example) and
    I decided not to fix them - I believe fixing them lead us to lots of repeating spacings issues.

@ethanshar ethanshar changed the title initial creation of the new TextField under incubator The new TextField (Incubator) Jul 16, 2020
@egorshulga
Copy link

Hey guys! I found out TextField is typed as any in the current release. Would this PR just replace the old one, would new TextField have correct typings, so it would be unnecessary to fix the old one?

@ethanshar
Copy link
Collaborator Author

Hi @egorshulga
Yes, once released the new TextField will include typings and better API overall.

@ethanshar ethanshar marked this pull request as ready for review August 10, 2020 10:39
@ethanshar ethanshar requested a review from Inbal-Tish August 10, 2020 10:40

export interface CharCounterProps {
showCharCounter?: boolean;
maxLength: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want to revert the change here and keep it optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed it back to optional
and moved the check for maxLength to CharCounter

@ethanshar ethanshar requested a review from Inbal-Tish August 18, 2020 05:43
@Inbal-Tish Inbal-Tish merged commit 1e86e5f into master Aug 18, 2020
@ethanshar ethanshar deleted the feat/NewTextField branch September 21, 2020 08:02
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.

3 participants