search

department-of-veterans-affairs / vets-design-system-documentation

Repository for design.va.gov website
https://design.va.gov
38 stars 58 forks source link

Incorrect role on <dd> element on "On this page" component #1998

Open laflannery opened 1 year ago

laflannery commented 1 year ago

Bug Report

What happened

The "On this page" component has a role="definition assigned to the <dd> element: image

I am asking that

  1. If there is a very good reason we are using the definition list markup:
    • The role be removed from this element because:
      • As with all ARIA, we do not want or need to use a role when a semantic element is available
      • Also, because we are using a role that is not aligned with the semantic element we are using, the implicit value of the element is changing and this is being flagged as an error in AMP as an incorrect element being used within the <dl> image
  2. However, this markup seems complex for this, it's just a navigation element and there doesn't seem to be a reason why this wouldn't just use a simply list markup.
    • Recommendation to update this to a typical list markup and remove the definition list entirely

What I expected to happen

I would expect the markup to use only expected elements and to use the least complex markup appropriate for the content and information you are trying to convey

Reproducing

Steps to reproduce:

1. 4. 5. 6.

Urgency

How urgent is this request? Please select the approriate option below and/or provide details

Details

caw310 commented 2 months ago

Hey team! Please add your planning poker estimate with Zenhub @Andrew565 @ataker @harshil1793 @it-harrison @jamigibbs @micahchiang @nickjg231 @powellkerry @rmessina1010 @rsmithadhoc

jamigibbs commented 2 months ago

I think I agree with @laflannery about the existing dl markup. Looking over MDN, the description list doesn't seem appropriate. Maybe it was initially done that way because it could maybe be described as a glossary? But that doesn't seem exactly right for this use case either. Unfortunately I could not find any details about it when it was created so I'm not sure the original reason.

Here are the guidelines for nav which can contain any type of markup but a list is probably best here.

Andrew565 commented 11 hours ago

My plan is to convert this component to use a UL and LI elements.