fix: corrects instances of nested unordered lists in ActionList
Hi team --
I'm opening this as a conversation starter. After a discussion regarding a Deque linting error around the markup ul.ul being a violation of permissible children for an ul, we've determined that the proper syntax is instead ul.li.ul.
I can think of a multi-pronged approach here:
- Change the docs to guide people
- Add linting to prevent providing
ul.ulto the component (if that's the API? I'm unfamiliar) - And, anything else we need to do as actionable after the conversation.
cc @khiga8 @inkblotty I'm not sure who should own this, hence the suggestion arriving at the code instead of as an issue somewhere.
What are you trying to accomplish?
An accessible-by-default ActionList component that's easy to use!
What approach did you choose and why?
TBD with the Primer team.
What should reviewers focus on?
Can these changes ship as is?
- [ ] Yes, this PR does not depend on additional changes. 🚢
⚠️ No Changeset found
Latest commit: b454dde735fb528a394a76c616bc1fe8cc0f2a3d
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
@andrialexandrou sorry I totally missed this notification!
~~It looks like this error will also need to be resolved here in primer/experimental/navigation_list/item.html.erb which seems to be actively in use.
@primer/rails-reviewers where can issue be filed?~~
Nvm, I think the nesting there is correctly:
ul li or
ul li ul.
Gonna cc: @langermank as I think you worked on this page and may know places this may affect.
Hi all! This markdown file is hidden from our public docs, and its pretty out of date. It was the initial draft of how the ActionList API should look. We're currently porting this CSS to be a PVC component, where it will have proper documentation. I think this particular bit of markup is human error, not an actual problem with the markup used in dotcom (please correct me if I'm wrong here). I'll remove this piece of documentation once the PVC component is complete.