When testing bug 40655, I noticed that while there is an alert that warns you if you enter an invalid value in a cell "Cost must be expressed as a decimal number >= 0" it only runs onblur, but you can still submit the form with invalid values. If you submit the form with an empty cell, you get an error "There were problems with your submission Invalid value for Centerville -> Fairfield" and the cell does not render an input anymore. It also let's you enter non numeric values which could cause unexpected results. To recreate: 1. Enable UseTransportCostMatrix and go to Administration > Transport cost matrix 2. Click in any cell 3. Uncheck the disabled box (don't click in the cell) 4. Click save 5. Notice the error "There were problems with your submission Invalid value for Centerville -> Fairview" 6. Notice the form is broken for that cell
Created attachment 187409 [details] [review] Bug 40948: Add form validation to transport cost matrix
Attaching this patch for feedback. It replaces the js alert with html form validation using the pattern attribute and a regular expression. It sets the input as required so you cannot submit an empty value. The pattern matches numeric values with up to two decimal places. Allows: 0 - zero 100 - integers 0.5 - leading 0 before a demical place .5 - single decimal place 20.75 - two decimal places Rejects: 001 - leading zeros before integer 10. - no digits after decimal 5.255 - three decimal places I also thought about making this a number type input with step=".01" Custom validation with Bootstrap might be nice too, but this was the simplest solution.
Another thought is that we could still trigger the form validation on blur of the input so that the user would get immediate feedback for the field as soon as they finish editing it like: $("body").on("blur", ".cost_input", function () { if ( ! $('#cost_matrix_form')[0].checkValidity() ) { $('#cost_matrix_form')[0].reportValidity(); } });
Brendan, is this ready for testing?
(In reply to Lucas Gass (lukeg) from comment #4) > Brendan, is this ready for testing? I didn't set this to needs sign off only because I'm not sure if this is the best solution, but it does work. From a ux point of view I think the onblur listener actually is a good idea, especially since a user could be editing many cells incorrectly before they submit the form. So I had the thought to add the onblur listener back, but instead of the alert it could check if the form is valid. The matrix is a tricky form because it's so dense. I'm not sure if the default html validation pop ups function as well here as they do on a more normal looking form, but it does the basic job of preventing unwanted values. I'd appreciate any feedback to consider the best way forward with this one. Thanks!