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

Hide error message when value changes #12759

Closed

Conversation

minkyngkm
Copy link
Contributor

@minkyngkm minkyngkm commented Apr 5, 2023

Done

  • Hide error message when value changes

QA

  • Go to /pro/subscribe
  • Click "Buy now" button
  • See the global error message displays at the top and validate message on the fields
  • Update some of the fields that show validate message
  • See the global error message disappears right after updating field
  • Type invalid vat number and check global error message for the vat number displays properly
  • Type invalid card number and check global error message displays properly
  • QA with the other error cases as well

Issue / Card

Fixes #https://warthogs.atlassian.net/jira/software/c/projects/WD/boards/801?modal=detail&selectedIssue=WD-2941

@webteam-app
Copy link

Demo starting at https://ubuntu-com-12759.demos.haus

@codecov
Copy link

codecov bot commented Apr 5, 2023

Codecov Report

Merging #12759 (6a18d6d) into main (83b1197) will decrease coverage by 0.13%.
The diff coverage is 16.66%.

@@            Coverage Diff             @@
##             main   #12759      +/-   ##
==========================================
- Coverage   73.68%   73.55%   -0.13%     
==========================================
  Files         104      104              
  Lines        2709     2715       +6     
  Branches      840      842       +2     
==========================================
+ Hits         1996     1997       +1     
- Misses        685      690       +5     
  Partials       28       28              
Impacted Files Coverage Δ
.../checkout/components/UserInfoForm/UserInfoForm.tsx 60.29% <0.00%> (-0.45%) ⬇️
...tage/subscribe/checkout/components/Taxes/Taxes.tsx 70.75% <20.00%> (-2.52%) ⬇️

@minkyngkm minkyngkm force-pushed the remove-error-message-on-change branch 2 times, most recently from d15cbbf to 847f00a Compare April 5, 2023 12:15
@minkyngkm minkyngkm force-pushed the remove-error-message-on-change branch from 847f00a to 9fcd10a Compare May 11, 2023 10:23
@minkyngkm minkyngkm force-pushed the remove-error-message-on-change branch from 9fcd10a to 6a18d6d Compare May 11, 2023 12:07
Copy link
Contributor

@albertkol albertkol left a comment

Choose a reason for hiding this comment

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

I don't like this implementation. It touches too many components.

I was hoping Formik has a onChange handler. I googled a bit and found this: https://plainenglish.io/blog/how-to-listen-to-formik-onchange-event-in-react
But the FormObserver, did not work as expected.

But rather than a component, perhaps having just a simple JS event listener for all input and select HTML elements that would hide the notification once it is displayed is enough.

Either way.. I would rather always have the notification up than add all these new changes to the components.

@minkyngkm
Copy link
Contributor Author

Closing this PR as the ticket has been moved into ice box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants