Make dates accessible

[plocket]

We do have labels for each of our own inputs, but I just realized that we don't have a matching name for the whole field label's for. I'm not sure exactly how to juggle that - can we move the label that's on the original date field, or does that break submitting the data? Should we change the field label's for to something we create ourselves? Do we need a legend to contain stuff to help the label be associated with those multiple fields? Maybe look at how checkboxes are made accessible.

[BryceStevenWilley]

Related to #214

[plocket]

By that, I'm guessing we don't need to associate the original date field with that group of fields, right?

[BryceStevenWilley]

I don't think so, they just need to be clearly labeled (i.e. all of the fields should still be related to the original field label somewhere, but we don't care about the original field itself)

[plocket]

Sorry I'm not being clear. That's the exact question I'm asking. I'm not sure we're currently associating each field with the original field label. We're associating them with their own labels - "month", "day", and "year", but I'm not sure that we're associating them with their main label, e.g. "The date you bought the house"

[BryceStevenWilley]

Ah, I see. I do think we want that; otherwise if there are multiple date inputs on a page, there will be multiple fields with the same labels ("Month", etc.), which can cause issues with accessibility tools that list all of the fields on a page and such.

I talk about bit more about how this could be done in #230 (review)

[plocket]

The most specific section from those comments (since I think this is possibly a harder problem to solve and would rather keep the discussion in here):

I think that we could make the whole widget a fieldset, with that label becoming the legend. I manually edited that HTML in the browser tools, and it's more clearly associated with the inputs, which is great, essentially just changing the div.da-field-container-datatype-ThreePartsDate to a fieldset, and changing the label tag to a legend. My HTML looks like this:

<fieldset class="da-container da-form-group row da-field-container da-field-container-datatype-ThreePartsDate">
    <legend for="ZGF0ZV9vZl9ncmFkdWF0aW9u" class="col-md-4 col-form-label da-form-label datext-right">
        When will your high school graduation be?
    </legend>
    <div class="col-md-8 dafieldpart">
        (unchanged)
        ...
</fieldset>

I my opinion, this will require a dive into da's core code, but I'm really interested in further thoughts and discussion.

I do believe these multiple fields should be in a fieldset. The main concern I have there is that there may be things like event listeners or styles dependent on that div and its attributes. If we change that element or hide it, we risk serious side effects. For example, right now, that element or its attributes might be used to scroll to an erroring element after submit. That's only one example that might cause an explicit error.

A possible option is to wrap the whole thing, including that div in the fieldset, etc, but we still couldn't remove the div's for attribute in case that's being used in an event listener and might trigger an error without risking current or future dependencies.

If we do dive into da's core and find that it actually is flexible enough, whatever we find in there may not be stable so we'd have to watch that constantly to detect any changes in any related code in case it adds new dependencies.

To handle this upstream is also potentially complicated because upstream code doesn't really touch or interact with custom datatypes. It's possible we could create that kind of relationship, but it's likely to be complex and I already get the feeling that custom datatypes isn't a well-loved addition, so I'm not sure how likely that is to be accepted. I/we can look into that at some point, though.

Also, I'm not sure that future core development will know/remember to use whatever we implement in order to work with that abstraction. The core code tends to favor working locally as opposed to using abstractions. It does use some abstractions, but also tends to duplicate code and use what's immediately around the desired functionality. Sorry, I'm not sure how to articulate it. Future development may just interact with the element itself, making assumptions that it's there, instead of using our abstraction.

Would love to hear more thoughts on this!

[BryceStevenWilley]

The main concern I have there is that there may be things like event listeners or styles dependent on that div and its attributes.

For this specific case, none of the styles change when manually changing the div to fieldset, label to legend, and removing the for attribute from the legend in the browser, and the page still scrolls to the correct erroring element on submit with those changes. Not everything, but it's all of the specific functionality you've mentioned.

More rigorously, of the classes on that div (da-container da-form-group row da-field-container), the only place that those classes are directly tied to div is a CSS ruleset for the label on a range datatype. Every other time I can see is actually generating the HTML code with that class, or a css ruleset on just the class, not a specific tag. It's a bit harder to 100% confirm for event listeners, but most of the event listeners that I see (searching addEventListener) are in bootstrap, which is fairly well coded to work with whatever tags you use.

whatever we find in there may not be stable so we'd have to watch that constantly to detect any changes in any related code in case it adds new dependencies.

This is already the case for everything else we do on docassemble. Breaking changes are the norm in docassemble, and we already have to deal with it. I understand that these changes might encroach a bit more on what DA will change without warning, but there has never been a hard and fast abstraction boundary for these things.

A possible option is to wrap the whole thing, including that div in the fieldset, etc, but we still couldn't remove the div's for attribute in case that's being used in an event listener and might trigger an error without risking current or future dependencies.

That is a good solution as well IMO, but we do still need to remove the for attribute. It's an accessibility error to have for point to an input that's not shown on the page. I want to say event listeners that get the element by it's for attribute are relying on semantic information about the page that's no longer true, and will likely be wrong behavior themselves, but you are right to think defensively. If it's an issue, that we could make a new legend with the same text, and hide the other label.

[plocket]

That's a ton of legwork, thanks for diving so deep. I have a general gut feeling that messing with the DOM should be done in core as much as possible and messing with it elsewhere is fragile. We've seen that multiple times in the current CustomDataType classes we've created. That said, this kind of stuff needs to be accessible and I can see that a new relationship with custom datatypes is probably a long way off. Le sigh.

Sounds like we'll go with further downstream changes to our custom datatype DOM structure. I'll try to get to that soon, but feel free to assign yourself to it if you want to take it on sooner.

Also, I reviewed ALKiln some more with this in mind and I think this shouldn't have an impact on test functionality. We can certainly update if needed.