Form validation does not respect required radio buttons

[trippetoe]

I have a simple BS Form, and i've set the 'required' property on a group radio button. When I click the Submit button without making a selection from the radio button group, the form does not display the expected 'required' message, and the onSubmit handler is called. I would expect the form to fail validation and the submit handler not to be called. It doesn't matter if i put 'required' on the form.element control or on the el.control.

The 'required' property does appear in the rendered markup:

A simplified version of the form

    <BsForm @formLayout="vertical" @model={{this}} @onSubmit={{this.submit}} as |form|>
      <form.element @controlType="radio" @label="Category" @property="radio" @options={{@model}} @optionLabelPath="name"
        as |el|>
        <el.control required {{on "change" this.onRadioButtonChange}} />
      </form.element>
      <form.submitButton>Submit</form.submitButton>
    </BsForm>

---

Output from ember bootstrap:info:

Npm packages:
ember-bootstrap: ^5.1.1 -> 5.1.1
ember-cli: ~4.7.0 -> 4.7.0      
bootstrap: ^5.2.2 -> 5.2.2
bootstrap-sass: n/a
ember-cli-sass: n/a
ember-cli-less: n/a
Bower packages:
bootstrap: n/a
bootstrap-sass: n/a
ember-bootstrap configuration:
bootstrapVersion: 5
importBootstrapCSS: true

[jelhan]

Thanks a lot for the detailed bug report. It would be helpful if you could provide a snapshot of the rendered HTML as well. Thanks.

[simonihmig]

Are you using https://github.com/ember-bootstrap/ember-bootstrap-constraint-validations, or just plain ember-bootstrap?

[trippetoe]

@simonihmig I am using plain ember-bootstrap

@jelhan I think this is the snapshot you were looking for. Let me if you would like to see anything else

[jelhan]

I think this is the snapshot you were looking for. Let me if you would like to see anything else

Sorry. I was not precise. I meant a snapshot of the HTML elements the Glimmer VM creates. The issue is likely that HTML is not as you expect.

[trippetoe]

Here is the HTML for the complete form (previously i shared only the radio button group). The radio button group starts at the label 'Category'

<div class="row mb-2">
  <div class="col-12">
    <div class="horizontal-section">
      <h1>Create an item</h1>
      <div class="right-aligned">
        <a id="ember119" class="ember-view" href="/items">
          Go back
        </a>
      </div>
    </div>
    <form>
      <div class=" mb-3 ">
  <label class="form-label" for="ember120-field">
<!---->  Name
</label>
        <input id="ember120-field" class="form-control  " placeholder="Enter the item's name" required="" type="text">
<!---->
<!----></div>

      <div class=" mb-3 ">
  <label class="form-label" for="ember121-field">
<!---->  Description
</label>
        <input id="ember121-field" aria-describedby="ember121-help" class="form-control  " placeholder="Enter the item's description" required="" type="text">
<!---->
<div id="ember121-help" class="form-text">
  Up to 100 characters
</div></div>

      <div class=" mb-3 ">
  <legend class="">
<!---->  Category
</legend>
        
            <div class="form-check">
      <input class="form-check-input" id="ember122-field-0" required="" type="radio">
      <label for="ember122-field-0" class="form-check-label">
            Application
      </label>
    </div>
    <div class="form-check">
      <input class="form-check-input" id="ember122-field-1" required="" type="radio">
      <label for="ember122-field-1" class="form-check-label">
            Hardware
      </label>
    </div>
    <div class="form-check">
      <input class="form-check-input" id="ember122-field-2" required="" type="radio">
      <label for="ember122-field-2" class="form-check-label">
            Software
      </label>
    </div>
    <div class="form-check">
      <input class="form-check-input" id="ember122-field-3" required="" type="radio">
      <label for="ember122-field-3" class="form-check-label">
            Project
      </label>
    </div>
<!---->
<!----></div>

      <legend>
        Tags
      </legend>
      <div class="d-flex flex-column mb-3">
          <span class="placeholder-tag-content">Select a category to see available tags.</span>
<!---->      </div>
      <button class="btn    btn-primary" type="submit">
  <!---->Submit
</button>
    
</form>

  </div>
</div>

[jelhan]

This seems to be caused by missing name attribute on the <input type="radio">.

Doing some tests I noticed that a form is submitted even if it has a <input type="radio" required>, which is not checked. See this JSFiddle as a demonstration: https://jsfiddle.net/1am79oju/

If a name attribute is added to the input, browser shows a validation error and does not submit the form. See this JSFiddle as a demonstration: https://jsfiddle.net/uvpd6mkc/

Radio inputs have a name attribute in Bootstrap documentation: https://getbootstrap.com/docs/5.3/forms/checks-radios/#radios Not having one seems to be a bug.

PR adding the name attribute is very welcome. As a workaround you could set the name attribute on the form control yourself.

[drc-admin]

If i am explicit about the 'name' attribute, required radio buttons are respected. When i changed my template to the below, all worked as expected.
<el.control {{on "change" this.onRadioButtonChange}} @name="category" required/>

I poked around in the ember-bootstrap source code a bit, but i couldn't find where to add some routine that would make the name attribute required or default to some reasonable value, maybe like the property attribute. So, i may not be the ideal one to do the PR

[jelhan]

Glad to hear that you found a solution to unblock your case.

I think it's up to the form control to set it. I noticed that the name attribute is already set on the radio control:

ember-bootstrap/addon/components/bs-form/element/control/radio.hbs

Line 11 in 4ec513d

name={{@name}}

But a @name argument is not set on the yielded component:

ember-bootstrap/addon/components/bs-form/element.hbs

Lines 80 to 101 in 4ec513d

	{{#let
	(component
	(ensure-safe-component
	(bs-default
	@controlComponent
	this.controlComponent
	)
	)
	value=this.value
	id=this.formElementId
	type=this.controlType
	label=@label
	disabled=this.args._disabled
	readonly=this.args._readonly
	required=@required
	options=@options
	optionLabelPath=@optionLabelPath
	ariaDescribedBy=(if @helpText this.ariaDescribedBy)
	onChange=this.doChange
	validationType=this.validation
	size=@size
	) as |Control|}}

It seems that not setting a name attribute for radio control is a regression. Would be good to understand how it was working in earlier versions. And when this regression was added.

Defaulting to @property sounds reasonable to me. If @property is not set, we could fallback to a unique identifier.

This might be considered a breaking change. I feel it would be good to land it as part of upcoming major. Will add it to v6 tracking issue.

I don't have much time for open source currently. Does this input makes you feel comfortable continue working on this bug? I feel next steps are:

1. Understand when setting name attribute was dropped. Maybe testing different versions locally might be a good starting point.
2. Draft a PR setting @name argument again.
3. Investigate which control components consume a @name argument to understand the impact of this change.

[jelhan]

This might be considered a breaking change. I feel it would be good to land it as part of upcoming major. Will add it to v6 tracking issue.

I don't think we should treat it as a breaking change. It's adjusting the HTML markup rendered by Ember Bootstrap with the one expected by Bootstrap. And it fixes a bug with browser's form validation.

Setting the name attribute may break some apps. E.g. if they set required attribute on the <input type="radio"> but expect validation to not work. But in that case they would rely on internal implementation detail, which is not covered by public API contract.