Prevent negative values for stock on hand

Using browser validation. I didn't use model validation because the on_hand pseudo-attribute doesn't support it.

But.. it turned out to  not be so simple. Browser validation can't work if the field is hidden, and breaks the javascript. So now I made the javascript smarter, and the end result is more helpful I think.

app/views/admin/products_v3/_table.html.haml

@@ -106,7 +106,7 @@
                     = variant.on_demand ? t(:on_demand) : variant.on_hand
                   %div.on-hand__popout{ style: 'display: none;', 'data-controller': 'toggle-control', 'data-popout-target': "dialog" }
                     .field
-                      = variant_form.number_field :on_hand, 'aria-label': t('admin.products_page.columns.on_hand'), 'data-toggle-control-target': 'control', disabled: variant_form.object.on_demand
+                      = variant_form.number_field :on_hand, min: 0, 'aria-label': t('admin.products_page.columns.on_hand'), 'data-toggle-control-target': 'control', disabled: variant_form.object.on_demand
                       = error_message_on variant, :on_hand
                     .field.checkbox
                       = variant_form.label :on_demand do

app/webpacker/controllers/popout_controller.js

@@ -42,6 +42,12 @@ export default class PopoutController extends Controller {
   close() {
     // Close if not already closed
     if (this.dialogTarget.style.display != "none") {
+      // Check every element for browser-side validation, before the fields get hidden.
+      if (!this.#enabledDisplayElements().every((element) => element.reportValidity())) {
+        // If any fail, don't close
+        return;
+      }
+
       // Update button to represent any changes
       this.buttonTarget.innerText = this.#displayValue();
       this.buttonTarget.classList.toggle("changed", this.#isChanged());

spec/javascripts/stimulus/popout_controller_test.js

@@ -16,7 +16,7 @@ describe("PopoutController", () => {
       <div data-controller="popout">
         <button id="button" data-popout-target="button">On demand</button>
         <div id="dialog" data-popout-target="dialog" style="display: none;">
-          <input id="input1" value="value1">
+          <input id="input1" value="value1" required>
           <label>
             <input id="input2" type="checkbox" value="value2" data-action="change->popout#closeIfChecked">
             label2
@@ -89,6 +89,14 @@ describe("PopoutController", () => {
       expect(input2.checked).toBe(false);
       expectToBeShown(dialog);
     });
+
+    it("doesn't close the dialog when a field is invalid", () => {
+      input1.value = "" // field is required
+
+      input4.click();
+      expectToBeShown(dialog);
+      // Browser will show a validation message
+    });
   });
 
   describe("Cleaning up", () => {

spec/system/admin/products_v3/products_spec.rb

@@ -181,10 +181,16 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do
         fill_in "Name", with: "Large box"
         fill_in "SKU", with: "POM-01"
         fill_in "Price", with: "10.25"
-        click_on "On Hand" # activate stock popout
-        fill_in "On Hand", with: "6"
+
+        click_on "On Hand" # activate popout
       end
 
+      # Stock popout
+      fill_in "On Hand", with: "-1"
+      click_button "Save changes" # attempt to save or close the popout
+      expect(page).to have_field "On Hand", with: "-1" # popout is still open
+      fill_in "On Hand", with: "6"
+
       expect {
         click_button "Save changes"
         product_a.reload