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 of Keyboard Shortcuts Dialog - empty li's, too many p's and div's #391

Closed
terracoda opened this issue Apr 10, 2018 · 5 comments
Closed
Assignees
Labels

Comments

@terracoda
Copy link
Contributor

terracoda commented Apr 10, 2018

While testing, I noticed some weird spacing in the A11y View, so I inspected the HTML for the keyboard shortcuts dialog.
Screen shot from inspector
screen shot 2018-04-10 at 12 29 29

A div element cannot be direct child of an ul element

It is ok if div elements contain things that need to stick together in the PDOM. It's important that they do not come between natural parent and child elements.

The lists in the Keyboard Shortcuts dialog should be like this:

<h2>Move or Jump Grabbed Balloon</h2>
<ul>
  <li>Move grabbed balloon up, left, down, or right with Arrow keys or with letter keys W, A, S, or D.</li>
  <li>Move slower with shift plus Arrow keys or Shift plus letter keys W, A, S, or D.</li>
  <li>Jump close to sweater with J plus S.</li>
  <li>Jump to wall with J plus W.</li>
  <li>Jump to near wall with J plus N.</li>
  <li>Jump to center with J plus C.</li>
</ul>

Here's the ideal fix:

  • The API should require that an li element is required to be a direct child of its parent ul
  • Use the li tag itself to wrap the content of the list item, rather than adding an extra p tag.
  • If you must use a p tag, it needs to go inside the li.
  • Same for the div elements. If possible, do not add extra divs. If not possible, the div and/or the p have to be inside the li.
  • If the purpose of the div is to "keep related content together for placing in the PDOM", it would be better to contain the parent ul, or contain the parent ul together with its labeling heading than to contain individual list items.

Additionally, removing the p should improvement the visual layout of the the bulleted lists in the A11y View:
screen shot 2018-04-10 at 12 58 53

@terracoda
Copy link
Contributor Author

terracoda commented Apr 10, 2018

This sim is in Dev Testing, so tagging phetsims/qa/issues/112.

@terracoda
Copy link
Contributor Author

@jessegreenberg, if you need me to provide a full example of the Keyboard Help dialog HTML, I can do that.

@jessegreenberg
Copy link
Contributor

Thanks @terracoda, I shouldn't need a full example, I think you already provided one earlier. This is caused by recent changes to the API, and usages of the API in the sim haven't been updated. Early versions of BASE did not have this problem.

@terracoda
Copy link
Contributor Author

Ok, understood. I thought that recent changes to API might be the cause.

@jessegreenberg
Copy link
Contributor

This was fixed as part of phetsims/scenery-phet#364, and the HTML is sound again (no divs, no ps, and lis are direct descendants of uls).

capture

Closing.

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