Bug 1717632 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I'm changing this to focus on the view/edit panel. I'm going to mostly leave keyboard controls to bug 1753093, and focus more on semantic issues here.

So, an update for the current design.

In "viewing" mode:

+ The content is within a `<form>` element. This is not appropriate in viewing mode because there are no exposed form controls. It should be in a "reading" element, like `<main>`.
+ The photo is not an `<img>` element, even though it should be. Plus it would need alt text along the lines of "Contact picture" and "Missing/No contact picture" depending on the state.
+ The `#nameOuter` element is structured `<h1>Contact name</h1><h2>Contact email</h2>` and is within a `<section id="detailsHeader">`.
  + This means that the heading's scope is only this otherwise empty `<section>`. They should be headings for the `<main>` element.
  + Moreover, the I'm not sure the email should count as a sub-heading because there is no other `h2` level heading. I think it is only meant to be part of the `<h1>` heading.
+ There is a non-hidden `<label class="screen-readers-only" for="displayName">` element in this section. Similar to the first point, we shouldn't be mixing form elements with the viewing page. Moreover, this element was set up to only appear on screen readers...
+ The toolbar should have `role="toolbar"` and have focus controls. This is a minor issue, and it probably makes sense to wait until we have a custom toolbar widget. See bug 1553231.
+ The `#detailsBody` element shouldn't be a `<section>`. It is only a container for sections.
+ The "active-time" element should probably be a `<time>` element.

In "editing" mode:

+ The photo button should be a `<button>` or an `<input type="file">`. I think it should also have a visible "Contact Picture" label, above or below it.
+ The photo image should be an `<img>` with alt text.
+ The photo "dialog" is not semantic at all, and there is no keyboard controls. I'm not sure we really need this separate dialog because it adds an extra step -- you have to click to set up a drop point and then drop the image -- when we could just accept the drop on the form itself. The photo button as a `<input type="file">` makes more sense to me.
+ The `#nameOuter` element has similar issues to above because they share the same DOM space.
  + The `<h2>` email title is still visible.
  + This should have the structure of a `<fieldset>`.
  + The `#displayName` input has no visible label: it is set as screen-reader-only even though all users need this info.
+ Initial focus is on the "First Name" input, even though "Display Name" is before it. Either the "Display Name" should be focused first, or it should appear after the name inputs.
+ The email addresses table.
  + See bug 1762126 comment 40. Note that the design I gave in bug 1762126 comment 40 is not the smoothest on a screen reader (at least on Orca), but the advantage is that the table structure provides some association between the adjacent form controls. A grid widget might improve this area, but it would require much more work.
  + Moreover, there are no visible labels in this section, but I think they would be useful for all users. I think the table headings should be made visible rather than screen-reader-only because it is the easiest way to read it. But I know this would be breaking the given design mock ups. As a compromise we could give each selector or input a `title` corresponding to the heading. 
+ The "Websites" and "Phone Numbers" similarly have multiple entries, but are structured differently to the "Email Addresses" section. They should be consistent and use a table as well. The above points would apply to them as well.
I'm changing this to focus on the view/edit panel. I'm going to mostly leave keyboard controls to bug 1753093, and focus more on semantic issues here.

So, an update for the current design.

In "viewing" mode:

+ The content is within a `<form>` element. This is not appropriate in viewing mode because there are no exposed form controls. It should be in a "reading" element, like `<main>`.
+ The photo is not an `<img>` element, even though it should be. Plus it would need alt text along the lines of "Contact picture" and "Missing/No contact picture" depending on the state.
+ The `#nameOuter` element is structured `<h1>Contact name</h1><h2>Contact email</h2>` and is within a `<section id="detailsHeader">`.
  + This means that the heading's scope is only this otherwise empty `<section>`. They should be headings for the `<main>` element.
  + Moreover, the I'm not sure the email should count as a sub-heading because there is no other `h2` level heading. I think it is only meant to be part of the `<h1>` heading.
+ There is a non-hidden `<label class="screen-readers-only" for="displayName">` element in this section. Similar to the first point, we shouldn't be mixing form elements with the viewing page. Moreover, this element was set up to only appear on screen readers...
+ The toolbar should have `role="toolbar"` and have focus controls. This is a minor issue, and it probably makes sense to wait until we have a custom toolbar widget. See bug 1553231.
+ The `#detailsBody` element shouldn't be a `<section>`. It is only a container for sections.
+ The "active-time" element should probably be a `<time>` element.

In "editing" mode:

+ The photo button should be a `<button>` or an `<input type="file">`. I think it should also have a visible "Contact Picture" label, above or below it.
+ The photo image should be an `<img>` with alt text.
+ The photo "dialog" is not semantic at all, and there is no keyboard controls. I'm not sure we really need this separate dialog because it adds an extra step -- you have to click to set up a drop point and then drop the image -- when we could just accept the drop on the form itself. The photo button as a `<input type="file">` makes more sense to me. EDIT: Just realised that the dialog seems to be mostly for cropping, and you can drop onto the form's contact, but the semantic issue remains.
+ The `#nameOuter` element has similar issues to above because they share the same DOM space.
  + The `<h2>` email title is still visible.
  + This should have the structure of a `<fieldset>`.
  + The `#displayName` input has no visible label: it is set as screen-reader-only even though all users need this info.
+ Initial focus is on the "First Name" input, even though "Display Name" is before it. Either the "Display Name" should be focused first, or it should appear after the name inputs.
+ The email addresses table.
  + See bug 1762126 comment 40. Note that the design I gave in bug 1762126 comment 40 is not the smoothest on a screen reader (at least on Orca), but the advantage is that the table structure provides some association between the adjacent form controls. A grid widget might improve this area, but it would require much more work.
  + Moreover, there are no visible labels in this section, but I think they would be useful for all users. I think the table headings should be made visible rather than screen-reader-only because it is the easiest way to read it. But I know this would be breaking the given design mock ups. As a compromise we could give each selector or input a `title` corresponding to the heading. 
+ The "Websites" and "Phone Numbers" similarly have multiple entries, but are structured differently to the "Email Addresses" section. They should be consistent and use a table as well. The above points would apply to them as well.
I'm changing this to focus on the view/edit panel. I'm going to mostly leave keyboard controls to bug 1753093, and focus more on semantic issues here.

So, an update for the current design.

In "viewing" mode:

+ The content is within a `<form>` element. This is not appropriate in viewing mode because there are no exposed form controls. It should be in a "reading" element, like `<main>`.
+ The photo is not an `<img>` element, even though it should be. Plus it would need alt text along the lines of "Contact picture" and "Missing/No contact picture" depending on the state.
+ The `#nameOuter` element is structured `<h1>Contact name</h1><h2>Contact email</h2>` and is within a `<section id="detailsHeader">`.
  + This means that the heading's scope is only this otherwise empty `<section>`. They should be headings for the `<main>` element.
  + Moreover, the I'm not sure the email should count as a sub-heading because there is no other `h2` level heading. I think it is only meant to be part of the `<h1>` heading.
+ There is a non-hidden `<label class="screen-readers-only" for="displayName">` element in this section. Similar to the first point, we shouldn't be mixing form elements with the viewing page. Moreover, this element was set up to only appear on screen readers...
+ The toolbar should have `role="toolbar"` and have focus controls. This is a minor issue, and it probably makes sense to wait until we have a custom toolbar widget. See bug 1553231.
+ The `#detailsBody` element shouldn't be a `<section>`. It is only a container for sections.
+ The "active-time" element should probably be a `<time>` element.

In "editing" mode:

+ The photo button should be a `<button>` or an `<input type="file">`. I think it should also have a visible "Contact Picture" label, above or below it.
+ The photo image should be an `<img>` with alt text.
+ The photo "dialog" is not semantic at all, and there is no keyboard controls. I'm not sure we really need this separate dialog because it adds an extra step -- you have to click to set up a drop point and then drop the image -- when we could just accept the drop on the form itself. The photo button as a `<input type="file">` makes more sense to me. EDIT: Just realised that the dialog seems to be mostly for cropping, and you can drop onto the form's photo, but the semantic issue remains.
+ The `#nameOuter` element has similar issues to above because they share the same DOM space.
  + The `<h2>` email title is still visible.
  + This should have the structure of a `<fieldset>`.
  + The `#displayName` input has no visible label: it is set as screen-reader-only even though all users need this info.
+ Initial focus is on the "First Name" input, even though "Display Name" is before it. Either the "Display Name" should be focused first, or it should appear after the name inputs.
+ The email addresses table.
  + See bug 1762126 comment 40. Note that the design I gave in bug 1762126 comment 40 is not the smoothest on a screen reader (at least on Orca), but the advantage is that the table structure provides some association between the adjacent form controls. A grid widget might improve this area, but it would require much more work.
  + Moreover, there are no visible labels in this section, but I think they would be useful for all users. I think the table headings should be made visible rather than screen-reader-only because it is the easiest way to read it. But I know this would be breaking the given design mock ups. As a compromise we could give each selector or input a `title` corresponding to the heading. 
+ The "Websites" and "Phone Numbers" similarly have multiple entries, but are structured differently to the "Email Addresses" section. They should be consistent and use a table as well. The above points would apply to them as well.

Back to Bug 1717632 Comment 5