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

Added promise debounce library (with updated typescript) and async validation in Catalog form #5284

Merged
merged 4 commits into from
Mar 13, 2019

Conversation

rvsia
Copy link
Contributor

@rvsia rvsia commented Feb 27, 2019

Description

  • Adds library for debouncing promises
  • Adds 'proof of concept' validation to Catalog form Services > Catalogs > Catalogs > Configuration > New/Edit (it controlls if name is available)
  • Updates data-driven-form renderer to suppor function validators
  • Updates Typescript to latest and allowed synthetic default export (to support the library)

Before
validationbefore

After
validationafter

@terezanovotna

@miq-bot miq-bot added the wip label Feb 27, 2019
@rvsia
Copy link
Contributor Author

rvsia commented Feb 27, 2019

@miq-bot add_label hammer/no, services, react, enhancement

@rvsia rvsia changed the title [WIP] Promise debounce and [WIP] Promise debounce and async validation in Catalog form Feb 27, 2019
@rvsia
Copy link
Contributor Author

rvsia commented Feb 27, 2019

@Hyperkid123

@terezanovotna
Copy link

Awesome!! great job!

@Hyperkid123
Copy link
Contributor

Hyperkid123 commented Feb 27, 2019

My only worry is that the author will stop supporting that package in near future. But the code seems simple enough to just steal it and move it mi MIQ 😄

We should also mention that if you want the validation to fail you need to return some value (for our components its the error message) from promise and if it should pass you return undefined.

Another thing that we should mention is that using promise based validator with normal function ones, the promise base will "break" them. Meaning that even if classic validator fails, the promise one will be completed when the promise is and that will most likely be after the normal ones which will render them useless. So your async validation will need some extra logic (adding required validation to promise resolution for instance). So what we could do is to actually export the validation functions from data driven forms and use them in promises. Or we can check if some validator is a promise and the hook rest of the validators to run when its resolved, but that another logic for something that will be used rarely.

@rvsia rvsia force-pushed the promise-debounce branch from 006693f to 7a1fc85 Compare March 1, 2019 15:25
@rvsia rvsia changed the title [WIP] Promise debounce and async validation in Catalog form Added promise debounce library with updated typescript and async validation in Catalog form Mar 1, 2019
@rvsia rvsia changed the title Added promise debounce library with updated typescript and async validation in Catalog form Added promise debounce library (with updated typescript) and async validation in Catalog form Mar 1, 2019
@miq-bot miq-bot removed the wip label Mar 1, 2019
@mzazrivec mzazrivec requested a review from Hyperkid123 March 6, 2019 06:45
Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

@rvsia The validation works but there is a small issue. The validator is triggers even if you change different form field. I would add some memoize function that will remember the last input value and if its the same as current just return the old error message. This way we won't be making extra requests.

Example here: https://codesandbox.io/s/wyq72qxly5
see the simpleMemoize function

Otherwise looks good.

@rvsia rvsia force-pushed the promise-debounce branch from 7a1fc85 to 7712e9a Compare March 6, 2019 13:13
@rvsia rvsia force-pushed the promise-debounce branch from 7712e9a to 60dd4eb Compare March 7, 2019 12:19
@rvsia
Copy link
Contributor Author

rvsia commented Mar 7, 2019

@Hyperkid123 Data-driven forms updated and it works correctly!

@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2019

Checked commits rvsia/manageiq-ui-classic@946234d~...60dd4eb with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@mzazrivec mzazrivec self-assigned this Mar 13, 2019
@mzazrivec mzazrivec added this to the Sprint 107 Ending Mar 18, 2019 milestone Mar 13, 2019
@mzazrivec mzazrivec merged commit d1f2f07 into ManageIQ:master Mar 13, 2019
@rvsia rvsia deleted the promise-debounce branch September 17, 2019 12:02
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.

5 participants