Implemented date time fields #82
Conversation
|
I suppose failing tests are because of missing DavyJonesLocker/client_side_validations#784 in old gemfiles |
| const oldWrappers = $.extend({}, ClientSideValidations.formBuilders['SimpleForm::FormBuilder'].wrappers) | ||
|
|
||
| // It would be probably better to use some stub library but I want to keep it simple | ||
| let customWrapperCalled = false; |
dist/simple-form.bootstrap4.esm.js
Outdated
| @@ -42,6 +45,46 @@ ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = { | |||
| element.removeClass('is-invalid'); | |||
| errorElement.remove(); | |||
| } | |||
| }, | |||
|
|
|||
| get horizontal_multi_select() { | |||
There was a problem hiding this comment.
I've checked get support and we should be fine with IE >= 9
But is it possible to use the same approach as before?
I mean horizontal_multi_select: function() { } and make it call multi_select with the same params
There was a problem hiding this comment.
Not sure what do you mean by "before". What you suggest would require change in
wrapper: function (name) {
return this.wrappers[name] || this.wrappers.default
},
which assumes wrapper key to be value not a function.
There was a problem hiding this comment.
Sorry, I meant "as the others".
which assumes wrapper key to be value not a function.
Got it.
Entirely my fault, but I don't like to see that get there.
I'm fine to set aliases at the end of the file if there is no other approach
const simpleFormFormBuilder = {
// ...
}
simpleFormFormBuilder.wrappers.horizontal_multi_select = simpleFormFormBuilder.wrappers.multi_select
simpleFormFormBuilder.wrappers.vertical_multi_select = simpleFormFormBuilder.wrappers.multi_select
ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = simpleFormFormBuilderThere was a problem hiding this comment.
Ok, refactored it as you suggested. I just thought with get it would have more clarity.
Yes, you should edit appraisals, as well as the travis configuration. Also, this change will work only on compatible versions of CSV, please ignore travis for the moment. Also, I'm more interested in finishing CSV's side before this. I've a lot of concerns but at the moment I don't have time to elaborate (long story very short: javascript breaks the form removing html fields, should not happen) |
…y CSV Javascript form builder for adding/removing of error messages. Before CSV used only form-wide wrapper but field may use custom wrapper specified:
- by field's `wrapper` attribute
- by field's type and `wrapper_mappings` attribute
- by field's type and SimpleForm config `wrapper_mappings`
MichalRemis
force-pushed
the
implemented-date-time-fields
branch
from
c223946
to
d40e9ae
Compare
dist/simple-form.bootstrap4.esm.js
Outdated
| @@ -42,6 +45,46 @@ ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = { | |||
| element.removeClass('is-invalid'); | |||
| errorElement.remove(); | |||
| } | |||
| }, | |||
|
|
|||
| get horizontal_multi_select() { | |||
There was a problem hiding this comment.
Sorry, I meant "as the others".
which assumes wrapper key to be value not a function.
Got it.
Entirely my fault, but I don't like to see that get there.
I'm fine to set aliases at the end of the file if there is no other approach
const simpleFormFormBuilder = {
// ...
}
simpleFormFormBuilder.wrappers.horizontal_multi_select = simpleFormFormBuilder.wrappers.multi_select
simpleFormFormBuilder.wrappers.vertical_multi_select = simpleFormFormBuilder.wrappers.multi_select
ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = simpleFormFormBuilder| @@ -12,15 +12,16 @@ QUnit.module('Validate SimpleForm', { | |||
| dataCsv = { | |||
| html_settings: { | |||
| type: 'SimpleForm::FormBuilder', | |||
| error_class: 'error small', | |||
There was a problem hiding this comment.
small presence had a reason:
Unfortunately this commit didn't come with a test, so we are not explicitly testing this behavior. 🙏🏼
There was a problem hiding this comment.
Ah I see purpose of small is to test if everything works if 2 classes are present. Sorry, I put it back. I was just comparing it with default simple_form setting and thought it should be same.
There was a problem hiding this comment.
Don't worry, the problem is the missing test, or at least a comment should be present
There was a problem hiding this comment.
Implemented an explicit test for this use case in CSV: DavyJonesLocker/client_side_validations@a49dd74
Will do the same here
Implemented date/time inputs for Bootstrap4 configuration. (skipped plain SimpleForm because it is complicated - it doesn't have special wrapper for date/time fields and neither marks invalid fields 'is-invalid' when server side validating)
This PR requires DavyJonesLocker/client_side_validations#784 and #81 to work. To make tests pass I included #81 commit in branch and required DavyJonesLocker/client_side_validations#784 branch in Gemfile. Not sure if this is correct approach