thedevdojo / voyager

Voyager - The Missing Laravel Admin
https://voyager.devdojo.com
MIT License
11.75k stars 2.67k forks source link

BREAD Field labels have wrong for attribute #4247

Open hitech95 opened 5 years ago

hitech95 commented 5 years ago

Version information

This is a code issue / error. It is present on the current "master" branch (1.2)

Description

The form label for attribute is not matched with the name field attribute. It is hardcoded: https://github.com/the-control-group/voyager/blob/7835b366babf63940aef18e52615852eb17e2651/resources/views/bread/edit-add.blade.php#L72

This is my suggestion/fix. <label class="control-label" for="{{ $row->field }}">{{ $row->display_name }}</label>

If you prefer a PR just ask.

Expected behavior

Labels should be matched with the corect form field.

emptynick commented 5 years ago

There is no id on the inputs which the label could refer to, so I guess we can just remove the for attribute completely.

mpge commented 3 years ago

There is no id on the inputs which the label could refer to, so I guess we can just remove the for attribute completely.

I mean, what difficulty arises from adding a ID to each of the inputs so the HTML is semantic. Could just use the same value as the name attribute.

I did run into this issue as well trying to do custom Javascript modifications.

mpge commented 3 years ago

In the meantime, I did write some quick JS to fix this on-load, just so my own modifications would work. Feel free to use.

When I get a chance I'll attempt to fix this the right way, but for now this may be the best option. It's not fully tested so beware of that.

$('.control-label').each(function() {
      if($(this).attr('for') == 'name') {
        // Find the closest form-control, grab the name and bind to id.
        var closestFormControl = $(this).closest('.form-group').find('.form-control');
        var closestFormControlName = closestFormControl.attr('name');
        var newNameFixName = 'field_' + closestFormControlName;

        closestFormControl.attr('id', newNameFixName);

        $(this).attr('for', newNameFixName)
      }
});