| # |
Apr 30th 2010, 15:24 |
jose_zap |
I think |
| # |
Apr 30th 2010, 15:24 |
jose_zap |
I'll commit the fix |
| # |
Apr 30th 2010, 15:24 |
ADmad |
mine ? |
| # |
Apr 30th 2010, 15:23 |
jose_zap |
The test case has a parenthese in a bad place |
| # |
Apr 30th 2010, 15:23 |
ADmad |
the suspense is killing me :) |
| # |
Apr 30th 2010, 15:23 |
jose_zap |
ADmad: I found why there were some funky thing in requiredField() |
| # |
Apr 30th 2010, 15:19 |
ADmad |
right |
| # |
Apr 30th 2010, 15:19 |
jose_zap |
magic |
| # |
Apr 30th 2010, 15:19 |
jose_zap |
it already does too much maginc |
| # |
Apr 30th 2010, 15:18 |
ADmad |
jose_zap: cool. lets see if he comes up anything else. as long as a field is not incorrect tagged as required i am fine with it. as for adding the required class we should stick to the pessimistic approach we currently have |
| # |
Apr 30th 2010, 15:16 |
jose_zap |
I think the same way |
| # |
Apr 30th 2010, 15:15 |
ADmad |
so there are possible grey areas and i think the code is fine the way it is |
| # |
Apr 30th 2010, 15:15 |
jose_zap |
yeah |
| # |
Apr 30th 2010, 15:14 |
ADmad |
*he should use |
| # |
Apr 30th 2010, 15:14 |
ADmad |
so if user wants to be sure of having the required class he use use the notEmpty rule imo |
| # |
Apr 30th 2010, 15:13 |
ADmad |
hmm yes it does in this case, but there can be other cases like say custom regex where empty is a valid value |
| # |
Apr 30th 2010, 15:12 |
jose_zap |
but allowEmpty does imply it, doesn't it?¡ |
| # |
Apr 30th 2010, 15:12 |
ADmad |
it doesnt add the 'required' class because there is no 'notEmpty' rules |
| # |
Apr 30th 2010, 15:12 |
jose_zap |
no? |
| # |
Apr 30th 2010, 15:11 |
ADmad |
actually no |
| # |
Apr 30th 2010, 15:11 |
ADmad |
right it doesnt fail when it should |
| # |
Apr 30th 2010, 15:10 |
ADmad |
checking |
| # |
Apr 30th 2010, 15:10 |
jose_zap |
well it does not fail in my box |
| # |
Apr 30th 2010, 15:10 |
jose_zap |
it should fail, right? |
| # |
Apr 30th 2010, 15:10 |
jose_zap |
ADmad: try this, in the test you just have made... change allowEmpty => true for allowEmpty => false |
| # |
Apr 30th 2010, 15:09 |
jose_zap |
ok, fine |
| # |
Apr 30th 2010, 15:09 |
jose_zap |
And I'm not that sure becasue debugging the requiredFiled() method I have found some weird things |
| # |
Apr 30th 2010, 15:09 |
ADmad |
i already closed it. We can reopen if needed :) |
| # |
Apr 30th 2010, 15:08 |
jose_zap |
I want to be 100% sure |
| # |
Apr 30th 2010, 15:08 |
jose_zap |
haha |
| # |
Apr 30th 2010, 15:08 |
jose_zap |
I'm still not convinced |
| # |
Apr 30th 2010, 15:04 |
ADmad |
ok so good to close the ticket ? |
| # |
Apr 30th 2010, 15:03 |
jose_zap |
it was the id that was failing |
| # |
Apr 30th 2010, 15:03 |
jose_zap |
it passes |
| # |
Apr 30th 2010, 15:03 |
jose_zap |
ADmad: my bad too |
| # |
Apr 30th 2010, 14:57 |
jose_zap |
ok |
| # |
Apr 30th 2010, 14:57 |
ADmad |
*div tag class |
| # |
Apr 30th 2010, 14:57 |
ADmad |
jose_zap: make sure you test case is failing for div tag and not something else |
| # |
Apr 30th 2010, 14:53 |
ADmad |
hehe |
| # |
Apr 30th 2010, 14:53 |
jose_zap |
and it does pass |
| # |
Apr 30th 2010, 14:53 |
jose_zap |
Your test case is exactly the same |