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.

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

🪟 🐛 Fix input validation on blur and cleanup signup error handling #14724

Merged
merged 7 commits into from
Jul 14, 2022

Conversation

edmundito
Copy link
Contributor

@edmundito edmundito commented Jul 14, 2022

What

Fixes #14727

This fixes an issue related to input validation not handling the focus/blur events correctly, causing the form validation messages to not appear when the form is set to validate on blur. Caused by #12398

Originally started as an issue with signup not showing errors, it also cleans up the error messages there as well.

How

  • The Input component has a custom onFocus onBlur events to handle highlight the input on focus/blur, but it needs to propagate the event if the parent is assigning these events, which Formik does through the Field component.
  • The LabeledInput component was a mess with the props and noticed this while inspecting the form so it's cleaned up
  • Updated i18n messages
  • Disabled validateOnChange in signup form

Recommended reading order

Top top bottom

@edmundito edmundito added area/frontend area/frontend Related to the Airbyte webapp labels Jul 14, 2022
@edmundito edmundito requested a review from a team as a code owner July 14, 2022 18:55
@github-actions github-actions bot added the area/platform issues related to the platform label Jul 14, 2022
onFocus={onInputFocusChange}
onBlur={onInputFocusChange}
onFocus={(event) => {
toggleFocused();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're touching this right now: Could we maybe change this useToggle into a useState and actually use onFocus/onBlur to set the focused state correctly. Most of the times it shouldn't be a problem, but I've encountered in the past some weird edge-cases where onFocus/onBlur might be called twice (e.g. when browser window focus changes in a weird way), in which case the toggle solution might lose correct track of actual focus.

Copy link
Collaborator

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested it locally, fixes the sign-up error issue. Small suggestion about switching the useToggle to useState to prevent some very edge-cases, but LGTM either way.

@edmundito edmundito merged commit 20150fa into master Jul 14, 2022
@edmundito edmundito deleted the edmundito/fix-signup-password-no-error branch July 14, 2022 20:34
jsrcodes added a commit to jsrcodes/airbyte that referenced this pull request Jul 15, 2022
…rbytehq-master

* 'master' of https://github.com/airbytehq/airbyte: (1141 commits)
  pass USE_STREAM_CAPABLE_STATE env var to containers/deployments (airbytehq#14737)
  Bump mqtt connector (airbytehq#14648)
  Add error code to ManualOperationResult (airbytehq#14657)
  Bump elasticsearch version (airbytehq#14640)
  Ryan/sync oracle version number (airbytehq#14736)
  Fixed linter issue with add_fields.py comments (airbytehq#14742)
  🎉Redshift, Databricks, Snowflake, S3 Destinations: Make S3 output filename configurable (airbytehq#14494)
  🐛Source-mssql: aligned regular and cdc syncs and its datatype tests (airbytehq#14379)
  🎉 Source Amazon Seller Partner: Add new streams (airbytehq#13604)
  bump source-file-secure (airbytehq#14704)
  🎉 New source: Timely airbytehq#13292 (airbytehq#14335)
  🪟🔧 Refactor feature service (airbytehq#14559)
  [low code cdk] add a transformation for adding fields into an outgoing record (airbytehq#14638)
  Bump destination-postgres to 0.3.21 (airbytehq#14479)
  Remove `additionalProperties: false` from JDBC destination connectors (airbytehq#14618)
  🎉 Source Notion: add OAuth authorization for source-notion connector (airbytehq#14706)
  Use the configuration diff calculation in the update endpoint (airbytehq#14626)
  🪟 🐛 Fix input validation on blur and cleanup signup error handling (airbytehq#14724)
  lower sleep after wait for successful job (airbytehq#14725)
  Add configuration diff (airbytehq#14603)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When entering invalid data into the input field, it does not show an error after unfocus
2 participants