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

known issue "Abide - custom validation slow" #6547 #6558

Merged
merged 4 commits into from Sep 3, 2015
Merged

known issue "Abide - custom validation slow" #6547 #6558

merged 4 commits into from Sep 3, 2015

Conversation

ghost
Copy link

@ghost ghost commented May 14, 2015

I fixed some 4 spaces to 2 spaces and some spelling errors in the code.
nothing major there. But I also fixed the abide to be faster by lowering
the check time from 1second to 1/10th of a second. This resolves the
feeling of lag and does not seem to hinder any other function that I am
seeing. This does not resolve the 3 times execution as of yet. but as of
now does resolve the speed issue.

caycek added 3 commits May 14, 2015 10:44
I fixed some 4 spaces to 2 spaces and some spelling errors in the code.
nothing major there. But I also fixed the abide to be faster by lowering
the check time from 1second to 1/10th of a second. This resolves the
feeling of lag and does not seem to hinder any other function that I am
seeing. This does not resolve the 3 times execution as of yet. but as of
now does resolve the speed issue.
see issues #4820, #4730

Will work for ALL equalto variations. So long they are input,textarea or
select. fix based on ID and no functionality was changed.
Fix a check to make sure that the element is not empty. The only reason
this check should be run is if it already has a value inside, otherwise
it is not necessary.
@ghost
Copy link
Author

ghost commented May 14, 2015

Trying to fix two problems here with abide... Speed was my first priority. Hopefully you feel this is fixed. I do not see any major drawbacks in the way that I'm using it, but cannot think of an issue which would cause it to be a poor push. Also, the fix with the equalTo check.... I just realized as I was typing this an error that could happen so that is the Hot fix. The check now goes back and finds any elements in the same form that have the id value set to X if so then it checks their value and so on so forth.

Questions or concerns please let me know.

might as well fix spelling errors while I'm at it...
@rafibomb
Copy link
Member

Love the PR! Thanks! Only thing we think should be changed is the timeout. We added a little documentation for it just now, we think the default should be left at 1s. It's a useful feature and not that it's pointed out in the docs (when we deploy) people can set it themselves.

@ghost
Copy link
Author

ghost commented May 15, 2015

That's understandable. I thought that might be the case! Do you want me to change it back to 1s or would you prefer to do that yourself?

Also, I could look into building an option into this build if you wanted me to. Not sure how that would work off the top of my head, but if not it can be left for 6 later. Let me know!

After checking further I see there is an option already! Let me know still if I need to do anything else.

Thanks!

@ghost
Copy link
Author

ghost commented Jun 5, 2015

@rafibomb Hey hope you're well. It has been some time since this was posted. I wanted to double check if / when you guys do accepting of pull requests. I did my branching incorrectly and only now finally understand how to do branching more correctly. If this has limited your ability to merge this request I can go through and reapply the changes or do what is needed on a separate branch.

Let me know when you can!

Thanks!

@rafibomb rafibomb added the abide label Jun 19, 2015
@rafibomb rafibomb added this to the 5.5.3 milestone Jun 19, 2015
@rafibomb
Copy link
Member

rafibomb commented Sep 3, 2015

We just need the conflicts cleared and we're good to go!

@rafibomb rafibomb assigned zurbchris and unassigned jamieshark Sep 3, 2015
@zurbchris zurbchris merged commit cfbda18 into foundation:master Sep 3, 2015
zurbchris added a commit that referenced this pull request Sep 3, 2015
@zurbchris
Copy link
Contributor

Thanks for the contribution!

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