Switch input labels to use <label> element
emplums opened this issue · 6 comments
Heya @hharnisc @alvarezmelissa87 @stevemdixon!
Just a heads up on an axe-core error I'm seeing while adding the tests - it looks like we're using a div element for the input labels instead of a <label>
const renderLabel = ({ label }) => (
label ? (
<div style={labelStyle}>
<Text>{label}</Text>
</div>
) : null
);
I wonder if instead of using a <Text>
component for this we could use the native <label>
component? That way screen readers have an easier time knowing which label is associated to which input.
Here's an example of how we might do that:
const renderLabel = ({ label }) => (
label ? (
<label for={id}>{label}</label>
) : null
);
The only thing tricky about this is that I believe we'll need a unique id for each instance of the input in order to associate the label with a particular <input>
I think it's time we create a Label
component 😄
@hharnisc agreed :) Do you have any ideas around creating unique id's to pass to the id tag?
@emily-plummer taking a quick look at the http://redux-form.com/6.8.0/ docs I think we get some help there 😄
So with each Field
we have to specify a name
prop. And that needs to be unique within each form. Each form needs a unique form
id.
unique name:
https://github.com/bufferapp/buffer-web-components/blob/master/DateTimeForm/index.jsx#L47
unique form:
https://github.com/bufferapp/buffer-web-components/blob/master/DateTimeForm/index.jsx#L88
I'm expecting it to be easy to grab the name
within the component, but not sure about the form id -- seems like it should be possible though. I've got some ideas how to get around this if we can't get the form though.
Does for
on the label
get read by the screen reader @emily-plummer? Need to understand that a little better before trying to implement this one I think 😄
@hharnisc I don't think so, the for
just lets the screen reader know which form element the label is associated with! An alternative way to associate a label with a form element is to wrap the form element with a label element like so:
return (
<div>
<Label label={label} isHidden={hideLabel}>
<input
style={style}
disabled={meta.submitting}
value={input.value}
onChange={input.onChange}
placeholder={placeholder}
type={type}
onFocus={onFocus}
onBlur={onBlur}
/>
</Label>
{renderError(meta)}
</div>
);
and then the label component would look like this:
return (
<label>
{label}
</label>
);
That would get around needing to grab an id for each input & would ensure that every input has a label! We can also pass an option hideLabel
that can visually hide the label if needed (but still make the label accessible to screen readers) - it would be ideal if all labels were visible but there may be some cases where there's enough context that a visual label might not be necessary (I'm thinking of dropdowns where the first item helps imply meaning like a dropdown of countries or state names)
Seems like a great idea to always have a label (visible or otherwise!). Pretty cool that the label can be associated with the input with nesting too!
I think it will keep things simpler if we use the for
and id
method since they're decoupled and give us some freedom around how to show and hide components.