Use HTML5 Validations for Frontend Checkout #1548 #1799
Conversation
| @@ -3,12 +3,12 @@ | |||
|
|
|||
| <p class="field"> | |||
| <%= label_tag "name_on_card_#{payment_method.id}", Spree.t(:name_on_card) %><span class="required">*</span><br /> | |||
| <%= text_field_tag "#{param_prefix}[name]", "#{@order.billing_firstname} #{@order.billing_lastname}", { id: "name_on_card_#{payment_method.id}", autocomplete: "cc-name" } %> | |||
| <%= text_field_tag "#{param_prefix}[name]", "#{@order.billing_firstname} #{@order.billing_lastname}", { id: "name_on_card_#{payment_method.id}", utocomplete: "cc-name", required: true } %> | |||
|
I am all in favour of this change 👍
|
| @@ -65,30 +67,31 @@ | |||
| class: !have_states ? 'required' : '', | |||
| style: have_states ? 'display: none;' : '', | |||
| disabled: have_states, | |||
| autocomplete: address_type + ' address-level1' | |||
| autocomplete: address_type + ' address-level1', | |||
| required: true | |||
There was a problem hiding this comment.
The required here should maybe follow the same logic as the required class. !have_states ? 'required' : ''.
Do we maybe need to remove the required class from those inputs as well? And use maybe the required attribute as a selector in the css if there's need.
There was a problem hiding this comment.
I don't think the !have_states? 'required' logic makes sense for anything else besides styles here. It's simple enough to remove the required: true attribute for a fringe case where address-level1 would not be required, but the form is for Debit/Credit or Shipping which do require an address.
| </p> | ||
|
|
||
| <p class="field" id=<%="#{address_id}phone" %>> | ||
| <%= form.label :phone, Spree.t(:phone) %><% if address.require_phone? %><span class="required">*</span><% end %><br /> | ||
| <%= form.phone_field :phone, class: "#{'required' if address.require_phone?}", autocomplete: address_type + ' home tel' %> | ||
| <%= form.phone_field :phone, class: "#{'required' if address.require_phone?}", autocomplete: address_type + ' home tel', required: true, pattern: '^([+]?\d{1,2}[-\s]?|)\d{3}[-\s]?\d{3}[-\s]?\d{4}$' %> |
There was a problem hiding this comment.
I think that the RegEx is too complicated to sit in the views, and we'd need tests for it, as at the moment I have no clue what its expected functionality would be.
In addition, any kind of validation in this field would have to consider any sort of telephone number that exists around the world, which I believe is a difficult job. Just recently I was to add an auto-format in this field for the US phone style, which would wrap the first 3 digits I believe it was in parentheses (123) ...
| @@ -12,7 +12,7 @@ | |||
| <% if @order.state == 'address' || !@order.email? %> | |||
| <p class="field" style='clear: both'> | |||
| <%= form.label :email %><br /> | |||
| <%= form.text_field :email %> | |||
| <%= form.text_field :email, required: true, pattern: "^[a-zA-Z0-9\.]+@[a-zA-Z0-9]+(\-)?[a-zA-Z0-9]+(\.)?[a-zA-Z0-9]{2,6}?\.[a-zA-Z]{2,6}$", title: "username@example.com" %> | |||
There was a problem hiding this comment.
Same thing as for the phone field is valid here - the logic is too complicated for the view, and it needs to be tested, so to understand what the expected validations are.
But before you go there, consider that there's already sort of an established RegEx in Solidus. See issue #1794 for more info.s
|
This can be closed since we merged this via #2264, thanks @gitmihalis ! |
Hey there. This is my first contribution to open source!
I removed the jQuery.validate plugin and added the HTML5 required attributes to the relevant input fields in checkout, as well as a couple of patterns to match basic phone number and email addresses. I don't think there are any new test to write seeing as none of the functionality has changed.
Looking forward to your feedback.