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

Async Validation and prevent activity auto closing when completed #13

Closed
wants to merge 13 commits into from

Conversation

thedumbtechguy
Copy link

Changed validation to use a callback therefore allowing for async validation
Removed finishing of activity when form is completed

fixes #12 and #11

@heinrichreimer
Copy link
Owner

Overall veeery nice pull request!!!
Just two suggestions:

  1. Move StepCheckerCallback in Step.java
  2. Give users the ability to use public void check(..., StepCheckerCallback callback) {} or public boolean check(...) {}. I'm not yet sure how to implement this but it should work with an abstract class. Any thoughts on this?
    Thank you!

@thedumbtechguy
Copy link
Author

No idea why, I almost missed the notifications. I will check and see what I can implement in the mean time.

@thedumbtechguy
Copy link
Author

As per your suggestions,

  1. Done
  2. I am thinking rather than changing the API totally like I have and having to bump the version number totally, I can make each Step have an additional constructor which accepts the original StepChecker. Then rename my implementation to StepCheckerAsync.
    The new constructor will wrap the StepChecker in a StepCheckerAsync making the internals consistent.
    That way, we allow the user the flexibility and maintain the original Major Version of 2 and bump it to 2.1.0.

added alternative constructors for StepCheckerAsync to allow for async validation
updated support dependencies
updated version codes and version names
@thedumbtechguy
Copy link
Author

Done. Kindly review.

Also, I think we need to move to the builder pattern. The constructors have become more cumbersome. If you do merge this pull, I will work on it.

@heinrichreimer
Copy link
Owner

Ok, really awsome changes. It presumably sounds like I'm a smart aleck, but I heve still some little points to mention:

  1. Move StepCheckerCallback into Step.java
  2. ProgressDialogs are a bit old-school and also userunfriendly and hard to style. Let's change that for a simple layout: It should be a FrameLayout and a centered, vertical orientated LinearLayout in it. In the LinearLayout there should be a horizontally centered ProgressBar and a horizontally centered TextView with a customizable "Loading..." text. Both the ProgressBar and the TextView should be colored based on the theme colors.
    Thumbs up for your work!

@heinrichreimer
Copy link
Owner

BTW, great way to achive consistency AND new features using StepChecker and StepCheckerAsync 👍

@heinrichreimer
Copy link
Owner

@frostymarvelous Builder pattern belongs to #8

@thedumbtechguy
Copy link
Author

Oh, I didn't notice the StepCheckerCallback was out again. I did the work in a separate branch ahead of master and realised this, so I basically just copied all the files into the master branch. Fixed.

Are you sure you want us to implement the progress? I only used the progress dialog as part of the demo.

@thedumbtechguy
Copy link
Author

Why I ask is because I feel the library should focus on doing what it does best. We will be incurring technical debt by adding unrelated features.

@heinrichreimer
Copy link
Owner

The ProgressBar should only be used if the validation is async. Either way progress indication and a 'completed'-screen are very common uses and therefore belong in this library IMHO. Would also be nice if we swap the ProgessBar with an ImageView showing a big check mark to indicate the end of the form. I await my summer holidays, I'll be able to work on the library then.

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.

Add Asynchronous validation
2 participants