Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML labels not bound to their form elements #325

Open
codeitdude opened this issue Oct 10, 2023 · 7 comments
Open

HTML labels not bound to their form elements #325

codeitdude opened this issue Oct 10, 2023 · 7 comments
Labels

Comments

@codeitdude
Copy link

Description

Hi, I forked the repository and will be submitting a commit for this issue soon. There are two options to resolve this issue, either wrap the form element in the <label>, or add a for attribute to the <label> and an id to the form element it should be bound to.

At a glance, you'd think wrapping the form element in the <label> would be the way to go, but that requires all the nested elements to be inline (i.e., no nested <div>s). So I've been adding for attributes and ids. Doing it this way avoids the nested block-level issue but doesn't include the fonticon in the <label>, so my solution there is to wrap the fonticons in their own <label>. It's perfectly fine to have multiple<label>s pointing to the same form element's id.

From what I can tell, only the email, password and remember me form elements are in duplicate on a given page, so I assigned topEmail, sideEmail and email as their ids, and the same for password and remember me, respectively. Does anyone know if there are any other duplicate element names used in the admin forms? I haven't gotten to those yet.

I have more modifications coming btw, namely WAI-ARIA. I'll be adding that in a subsequent commit.

LiteCart Version

2.5.4

PHP Version

8.1

Error/Backtrace

No response

Is this a problem that can be reproduced in the demo platform?

None

If this problem could be related to a browser. Which one?

None

@codeitdude codeitdude added the bug label Oct 10, 2023
@codeitdude codeitdude changed the title labels not bound to their form elements HTML labels not bound to their form elements Oct 10, 2023
@timint
Copy link
Collaborator

timint commented Oct 11, 2023

I am aware of this. To be able to bind them to the input field they would have to go after it.

input[name="field"] + label {
 ...
}
<div class="form-group">
  <input name="field">
  <label>Title</label>
</div>

I never adopted this as it requires some nasty CSS to get the label above the field. Like margin-top: -2em. If there is a better approach we can consider adopting this for 3.0. But it has to be clean and simple.

Edit: I am speed reading, and it's possible I misunderstood you. You would like to address the input fields from their labels by click, yes? Like <label><input></label> or <label for="field"></label><input id="field">. I spontaneously thought you wanted to address them by stylíng? What comes to duplicate elements there is no way of knowing. We could produce a javascript code that focuses the field upon clicking a label if that is the problem. But I personally never saw any of this as a problem.

@codeitdude
Copy link
Author

codeitdude commented Oct 11, 2023

Yes you did misunderstand at a glance, I meant <label for="field"></label><input id="field">. The way to avoid the duplicate element ids is to NOT address it in includes\functions\func_form.inc.php but where you currently have the <label>s, such as in includes\templates\default.catalog\pages\create_account.inc.php.

That would look like this:

<div class="form-group col-xs-6">
   <label for="company"><?php echo language::translate('title_company', 'Company'); ?> (<?php echo language::translate('text_or_leave_blank', 'Or leave blank'); ?>)</label>
   <?php echo functions::form_draw_text_field('company', true, ' id="company"'); ?>
</div>

My intention here is to make LiteCart more accessible. Not only does this make the text "Company" clickable to add focus to the input for company, it also helps accessibility screen readers convey to the visually-impaired that this is the input for company.

I've added a screen reader to my Chrome so I can test it.

@timint
Copy link
Collaborator

timint commented Oct 16, 2023

This is one way of doing it. But as label is natively an inline element, it feels wrong to use it as a block.

<label class="form-group">
  <div>Company</div>
  <input class="" type="text" name="company" value="" data-type="text">
</label>

@codeitdude
Copy link
Author

Hey brother, that works for the instances of input="checkbox", but few others instance because of the divs, yes. I have a commit for you, delayed because I'm going through my modifications line by line making sure I don't overwrite any of your changes from my current working version and the one you're modifying at the moment. Commit coming soon.

@timint
Copy link
Collaborator

timint commented Oct 16, 2023

It works for any input, select or textarea. I want to avoid having to set ids for each and every field and label.

@timint
Copy link
Collaborator

timint commented Oct 16, 2023

Here is a universal way using javascript I came up with. Not tested, may contain an error or two.

var fieldIndex = 0;

$(':input').each(function(){
  var $form_group = $(this).closest('.form-group'),
   $label = $form_group.find('label');
  
  if (!$form_group.length) return;
  if ($label.length != 1) return;
  if ($form_group.find(':input').length != 1) return;
  
  $label.attr('id', 'field-'+fieldIndex);
  $(this).attr('id', 'field-'+fieldIndex++);
});

@codeitdude
Copy link
Author

I've only modified the public facing pages. I came up with a similar javascript to give the fonticons label-like behavior, it turns out that while it's valid to have multiple labels pointing to the same ID, it's bad for screen readers, they read each label...

//this gives fonticons a behavior similar to the <label for="element"><input id="element"> without adding extra <label>s to the HTML

$(document).ready(function() {
    $(".font-icon-clickable").on("click", function() {
        var input = $(this).next("input");
        if (input.length > 0) {
            input.focus();
           if (input.val() !== "") {
               input.select();
            }
        }
    });
});

Link to each public facing page, html validation & page speed:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants