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.

Sign up for GitHub

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

Implemented date time fields #82

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Implemented date time fields #82

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
53f6325
Implemented field's specific wrapper_names to be passed to and used b…
MichalRemis Apr 14, 2020
d9b70c9
Add support for Date/Time inputs (bootstrap4)
MichalRemis Apr 27, 2020
5dbb4d4
Plain SimpleForm Date/Time WIP
MichalRemis Apr 27, 2020
d40e9ae
RemovePlainSimpleForm
MichalRemis Apr 28, 2020
32112c1
Refactored MultiSelect JS builder and fix test
MichalRemis May 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@

source 'https://rubygems.org'

gem 'client_side_validations', github: 'MichalRemis/client_side_validations', branch: 'SupportDateTimeSelects'

gemspec
47 changes: 45 additions & 2 deletions dist/simple-form.bootstrap4.esm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@ import ClientSideValidations from '@client-side-validations/client-side-validati

ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
add: function add(element, settings, message) {
this.wrapper(settings.wrapper).add.call(this, element, settings, message);
this.wrapper(this.wrapperName(element, settings)).add.call(this, element, settings, message);
},
remove: function remove(element, settings) {
this.wrapper(settings.wrapper).remove.call(this, element, settings);
this.wrapper(this.wrapperName(element, settings)).remove.call(this, element, settings);
},
wrapper: function wrapper(name) {
return this.wrappers[name] || this.wrappers["default"];
},
wrapperName: function wrapperName(element, settings) {
return element.data('clientSideValidationsWrapper') || settings.wrapper;
},
wrappers: {
"default": {
add: function add(element, settings, message) {
Expand All @@ -42,6 +45,46 @@ ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
element.removeClass('is-invalid');
errorElement.remove();
}
},

get horizontal_multi_select() {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

Ok, refactored it as you suggested. I just thought with get it would have more clarity.

return this.multi_select;
},

get vertical_multi_select() {
return this.multi_select;
},

multi_select: {
add: function add(element, settings, message) {
var wrapperElement = element.closest(settings.wrapper_tag + '.' + settings.wrapper_class.replace(/ /g, '.'));
var parentElement = element.parent();
var errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback');

if (!errorElement.length) {
errorElement = $('<' + settings.error_tag + '>', {
"class": 'invalid-feedback d-block',
text: message
});
parentElement.after(errorElement);
}

wrapperElement.addClass(settings.wrapper_error_class);
element.addClass('is-invalid');
errorElement.text(message);
},
remove: function remove(element, settings) {
var wrapperElement = element.closest(settings.wrapper_tag + '.' + settings.wrapper_class.replace(/ /g, '.'));
var errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback');
var invalidSiblingExists = element.siblings('.is-invalid').length;

if (!invalidSiblingExists) {
wrapperElement.removeClass(settings.wrapper_error_class);
errorElement.remove();
}

element.removeClass('is-invalid');
}
}
}
};
47 changes: 45 additions & 2 deletions dist/simple-form.bootstrap4.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@

ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
add: function add(element, settings, message) {
this.wrapper(settings.wrapper).add.call(this, element, settings, message);
this.wrapper(this.wrapperName(element, settings)).add.call(this, element, settings, message);
},
remove: function remove(element, settings) {
this.wrapper(settings.wrapper).remove.call(this, element, settings);
this.wrapper(this.wrapperName(element, settings)).remove.call(this, element, settings);
},
wrapper: function wrapper(name) {
return this.wrappers[name] || this.wrappers["default"];
},
wrapperName: function wrapperName(element, settings) {
return element.data('clientSideValidationsWrapper') || settings.wrapper;
},
wrappers: {
"default": {
add: function add(element, settings, message) {
Expand All @@ -48,6 +51,46 @@
element.removeClass('is-invalid');
errorElement.remove();
}
},

get horizontal_multi_select() {
return this.multi_select;
},

get vertical_multi_select() {
return this.multi_select;
},

multi_select: {
add: function add(element, settings, message) {
var wrapperElement = element.closest(settings.wrapper_tag + '.' + settings.wrapper_class.replace(/ /g, '.'));
var parentElement = element.parent();
var errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback');

if (!errorElement.length) {
errorElement = $('<' + settings.error_tag + '>', {
"class": 'invalid-feedback d-block',
text: message
});
parentElement.after(errorElement);
}

wrapperElement.addClass(settings.wrapper_error_class);
element.addClass('is-invalid');
errorElement.text(message);
},
remove: function remove(element, settings) {
var wrapperElement = element.closest(settings.wrapper_tag + '.' + settings.wrapper_class.replace(/ /g, '.'));
var errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback');
var invalidSiblingExists = element.siblings('.is-invalid').length;

if (!invalidSiblingExists) {
wrapperElement.removeClass(settings.wrapper_error_class);
errorElement.remove();
}

element.removeClass('is-invalid');
}
}
}
};
Expand Down
7 changes: 5 additions & 2 deletions dist/simple-form.esm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@ import ClientSideValidations from '@client-side-validations/client-side-validati

ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
add: function add(element, settings, message) {
this.wrapper(settings.wrapper).add.call(this, element, settings, message);
this.wrapper(this.wrapperName(element, settings)).add.call(this, element, settings, message);
},
remove: function remove(element, settings) {
this.wrapper(settings.wrapper).remove.call(this, element, settings);
this.wrapper(this.wrapperName(element, settings)).remove.call(this, element, settings);
},
wrapper: function wrapper(name) {
return this.wrappers[name] || this.wrappers["default"];
},
wrapperName: function wrapperName(element, settings) {
return element.data('clientSideValidationsWrapper') || settings.wrapper;
},
wrappers: {
"default": {
add: function add(element, settings, message) {
Expand Down
7 changes: 5 additions & 2 deletions dist/simple-form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@

ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
add: function add(element, settings, message) {
this.wrapper(settings.wrapper).add.call(this, element, settings, message);
this.wrapper(this.wrapperName(element, settings)).add.call(this, element, settings, message);
},
remove: function remove(element, settings) {
this.wrapper(settings.wrapper).remove.call(this, element, settings);
this.wrapper(this.wrapperName(element, settings)).remove.call(this, element, settings);
},
wrapper: function wrapper(name) {
return this.wrappers[name] || this.wrappers["default"];
},
wrapperName: function wrapperName(element, settings) {
return element.data('clientSideValidationsWrapper') || settings.wrapper;
},
wrappers: {
"default": {
add: function add(element, settings, message) {
Expand Down
10 changes: 10 additions & 0 deletions lib/client_side_validations/simple_form/form_builder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ def input(attribute_name, options = {}, &block)
options.delete(:validate)
end

add_field_specific_wrapper_name_to_field_options(attribute_name, options, &block)

super(attribute_name, options, &block)
end

Expand All @@ -34,6 +36,14 @@ def wrapper_error_component
wrapper.find(:full_error)
end
end

def add_field_specific_wrapper_name_to_field_options(attribute_name, options, &block)
wrapper_name = options[:wrapper] || find_wrapper_mapping(find_input(attribute_name, options, &block).input_type)
return if wrapper_name.nil?

options[:input_html] ||= {}
options[:input_html][:'data-client-side-validations-wrapper'] = wrapper_name
end
end
end
end
Expand Down
44 changes: 41 additions & 3 deletions src/main.bootstrap4.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@ import ClientSideValidations from '@client-side-validations/client-side-validati

ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
add: function (element, settings, message) {
this.wrapper(settings.wrapper).add.call(this, element, settings, message)
this.wrapper(this.wrapperName(element, settings)).add.call(this, element, settings, message)
},
remove: function (element, settings) {
this.wrapper(settings.wrapper).remove.call(this, element, settings)
this.wrapper(this.wrapperName(element, settings)).remove.call(this, element, settings)
},
wrapper: function (name) {
return this.wrappers[name] || this.wrappers.default
},
wrapperName: function (element, settings) {
return element.data('clientSideValidationsWrapper') || settings.wrapper
},

wrappers: {
default: {
add (element, settings, message) {
const wrapperElement = element.parent()
let errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback')
var errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback')

if (!errorElement.length) {
errorElement = $('<' + settings.error_tag + '>', { class: 'invalid-feedback', text: message })
Expand All @@ -36,6 +39,41 @@ ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
element.removeClass('is-invalid')
errorElement.remove()
}
},
get horizontal_multi_select () {
return this.multi_select
},
get vertical_multi_select () {
return this.multi_select
},
multi_select: {
add (element, settings, message) {
const wrapperElement = element.closest(settings.wrapper_tag + '.' + settings.wrapper_class.replace(/ /g, '.'))
const parentElement = element.parent()
var errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback')

if (!errorElement.length) {
errorElement = $('<' + settings.error_tag + '>', { class: 'invalid-feedback d-block', text: message })
parentElement.after(errorElement)
}

wrapperElement.addClass(settings.wrapper_error_class)
element.addClass('is-invalid')
errorElement.text(message)
},
remove (element, settings) {
const wrapperElement = element.closest(settings.wrapper_tag + '.' + settings.wrapper_class.replace(/ /g, '.'))
const errorElement = wrapperElement.find(settings.error_tag + '.invalid-feedback')

const invalidSiblingExists = element.siblings('.is-invalid').length

if (!invalidSiblingExists) {
wrapperElement.removeClass(settings.wrapper_error_class)
errorElement.remove()
}

element.removeClass('is-invalid')
}
}
}
}
9 changes: 6 additions & 3 deletions src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@ import ClientSideValidations from '@client-side-validations/client-side-validati

ClientSideValidations.formBuilders['SimpleForm::FormBuilder'] = {
add: function (element, settings, message) {
this.wrapper(settings.wrapper).add.call(this, element, settings, message)
this.wrapper(this.wrapperName(element, settings)).add.call(this, element, settings, message)
},
remove: function (element, settings) {
this.wrapper(settings.wrapper).remove.call(this, element, settings)
this.wrapper(this.wrapperName(element, settings)).remove.call(this, element, settings)
},
wrapper: function (name) {
return this.wrappers[name] || this.wrappers.default
},
wrapperName: function (element, settings) {
return element.data('clientSideValidationsWrapper') || settings.wrapper
},

wrappers: {
default: {
add (element, settings, message) {
const wrapper = element.closest(settings.wrapper_tag + '.' + settings.wrapper_class.replace(/ /g, '.'))
let errorElement = wrapper.find(settings.error_tag + '.' + settings.error_class.replace(/ /g, '.'))
var errorElement = wrapper.find(settings.error_tag + '.' + settings.error_class.replace(/ /g, '.'))

if (!errorElement.length) {
errorElement = $('<' + settings.error_tag + '>', { class: settings.error_class, text: message })
Expand Down
30 changes: 28 additions & 2 deletions test/javascript/public/test/form_builders/validateSimpleForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ QUnit.module('Validate SimpleForm', {
dataCsv = {
html_settings: {
type: 'SimpleForm::FormBuilder',
error_class: 'error small',
Copy link
Contributor

Choose a reason for hiding this comment

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

small presence had a reason:

399f389

Unfortunately this commit didn't come with a test, so we are not explicitly testing this behavior. 🙏🏼

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't worry, the problem is the missing test, or at least a comment should be present

Copy link
Contributor

Choose a reason for hiding this comment

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

Implemented an explicit test for this use case in CSV: DavyJonesLocker/client_side_validations@a49dd74

Will do the same here

error_class: 'error',
error_tag: 'span',
wrapper_error_class: 'field_with_errors',
wrapper_tag: 'div',
wrapper_class: 'input',
wrapper: 'default'
},
validators: {
'user[name]': { presence: [{ message: 'must be present' }], format: [{ message: 'is invalid', 'with': { options: 'g', source: '\\d+' } }] }
'user[name]': { presence: [{ message: 'must be present' }], format: [{ message: 'is invalid', 'with': { options: 'g', source: '\\d+' } }] },
'user[date_of_birth]': { presence: [{ message: 'must be present' }] }
}
}

Expand All @@ -40,6 +41,12 @@ QUnit.module('Validate SimpleForm', {
type: 'text'
}))
.append($('<label for="user_name">Name</label>'))
.append($('<input />', {
name: 'user[date_of_birth]',
id: 'date_of_birth',
type: 'text',
'data-client-side-validations-wrapper': 'custom_date_wrapper'
}))
$('form#new_user').validate()
}
})
Expand Down Expand Up @@ -82,3 +89,22 @@ QUnit.test('Validate pre-existing error blocks are re-used', function (assert) {
assert.ok(input.parent().find('span.error:contains("is invalid")').length === 1)
assert.ok(form.find('span.error').length === 1)
})

QUnit.test('Validate correct JS Builder\'s wrapper is called for custom_wrapper', function (assert) {
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, remember no ES6 here

Copy link
Author

Choose a reason for hiding this comment

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

Yup sorry, I cherry-picked this from #81, I guess I will remove this commit once you merge/implement #81. Fixed in both #81 and this PR.


ClientSideValidations.formBuilders['SimpleForm::FormBuilder'].wrappers['custom_date_wrapper'] = {
add: function(element, settings, message) { customWrapperCalled=true; },
remove: function(element, settings) {}
}

var form = $('form#new_user');
var input = form.find('input#date_of_birth')
input.trigger('focusout')

assert.ok(customWrapperCalled);
ClientSideValidations.formBuilders['SimpleForm::FormBuilder'].wrappers = oldWrappers
})
Loading