open-ui icon indicating copy to clipboard operation
open-ui copied to clipboard

Panelset/Spicy-Sections issue(s) review/rfc

Open bkardell opened this issue 2 years ago • 11 comments

We have been doing work in a branch of the tabvengers/spicy-sections repo preparing to get something closer to being able to be pulled over to open ui and "closer" to reality.

On the surface, it changes to and the IDL class is now similarly named. Beneath that though are also a number of reworks/changes we would like to discuss/consider.... We'd like to present and discuss roughly the following...

Pros:

  • The CSS is as we'd like it to be in CSS (or some CSS-like) - just a property, familliar, integratable with everything, even container queries
  • No attributes are spouted in the light dom. It is much closer to how a native element would appear from the outside and how we'd probably handle most things on the suface.
  • Things remain as stylable through parts and circularities avoided

Things we could use help evaluating:

Our previous approach tried to punt on the details of the CSS property and how it would work, etc and lean into something simple that avoided circuarity. This solution is better for devs, more understandable, but observes a CSS Custom Property. Our data and our testing suggest this is not a perf problem, doesn't cause recalcs and stuff wildly - but we'd like to be sure.

Previously there was very little Shadow DOM, things were simple pair based and in relation to the actual content in the light DOM. That is, if there was a heading, it's content was simply ~"the next sibling" as in any document naturally. With this approach we have to create (and keep in sync) slots for an unknown number of light DOM children. It's some more bookkeeping and we'd like to know if this creates spec/implementation hurdles.

Previously headings were treated as tabs sometimes, that is, they would change their role. Because ARIA doesn't allow dual roles and we're saying that it is simply a different presentation, that was one way to do it. In this prototype, headings are slotted into shadow dom - so you get a role tab that contains a role heading. Is that better/worse from an a11y point of view? Or about the same?

bkardell avatar Mar 16 '22 20:03 bkardell

Previously headings were treated as tabs sometimes, that is, they would change their role. Because ARIA doesn't allow dual roles and we're saying that it is simply a different presentation, that was one way to do it. In this prototype, headings are slotted into shadow dom - so you get a role tab that contains a role heading. Is that better/worse from an a11y point of view? Or about the same?

@bkardell Is there a demo with this?

jnurthen avatar Mar 16 '22 21:03 jnurthen

@jnurthen, there is a demo of the new functionality currently hosted at https://tabvengers.github.io/spicy-sections/demonstration/.

jonathantneal avatar Mar 16 '22 23:03 jonathantneal

Previously headings were treated as tabs sometimes, that is, they would change their role. Because ARIA doesn't allow dual roles and we're saying that it is simply a different presentation, that was one way to do it. In this prototype, headings are slotted into shadow dom - so you get a role tab that contains a role heading. Is that better/worse from an a11y point of view? Or about the same?

This makes things worse when in the tab view. The Heading hierarchy is wrong. In the demo page the "Tabs" heading is followed by 2 more headings rather than its content. Headings are not useful if their content does not immediately follow them.

jnurthen avatar Mar 16 '22 23:03 jnurthen

Should be resolvable by setting role none or presentation on those in that view. I guess that's gonna sprout an attribute for now but it should work, right? @jnurthen / @sundress

bkardell avatar Mar 17 '22 01:03 bkardell

In the demo page the "Tabs" heading is followed by 2 more headings rather than its content.

I’d like to understand this better. Is this something we can currently test?

Also, the demo has been moved to https://tabvengers.github.io/spicy-sections/demonstration/

jonathantneal avatar Mar 17 '22 04:03 jonathantneal

seems on initial load the demo has it in reverse for the selected tabs. "tabs", the current tab has aria-selected=false. The other two tabs have aria-selected=true

scottaohara avatar Mar 17 '22 11:03 scottaohara

I’d like to understand this better. Is this something we can currently test?

Both Chromium browsers and Firefox aren't treating children of the button as presentational, but Webkit still follows the spec and does, so the heading is 'hidden' with VO+Safari.

NVDA with Firefox and Chrome/Edge, or Narrator with Edge will also have access to the headings. JAWS seems to have their own logic in place to adhere to the spec, so JAWS continues to ignore the semantics of button children.

I know that Firefox/Chromium made this change to help ensure that authors messing up and nesting invalid elements wouldn't prevent people from accessing information - but situations like this are frustrating. The spec'd behavior doesn't match reality, and not everyone is on the same page. So in this instance, where you actually want the heading semantics dropped, now needs ARIA's role=none to get what used to be taken care of by default.

scottaohara avatar Mar 17 '22 12:03 scottaohara

Thanks for that information, @scottaohara.

I’ve fixed the initial aria-selected issue, and (for now) added role=none to headings in the lightdom when an affordance is applied.

If there is another element that removes descendant roles, I would prefer to place that into the ShadowDOM as a solution.

For panelset to be considered as an HTML element, would Chromium/Firefox browsers need to readdress this behavior or carve out an exception? Do you think this would be a potential blocker for the panelset element supporting labels from “good ol’ well supported HTML” headings?

jonathantneal avatar Mar 17 '22 16:03 jonathantneal

Tabs are one of the reasons chromium made this change for the case of delete buttons etc. being exposed on tabs so I don't see them reversing this on panelset. Perhaps they could change their logic to only expose interactive children?

jnurthen avatar Mar 17 '22 16:03 jnurthen

Perhaps they could change their logic to only expose interactive children?

that's likely exactly what should be done. I've spoken to JAWS about this concerning the details > summary hubabaloo and they did not seem very interested in the idea of not respecting child elements as presentational if the parent element was mapped to a button (but seem fine exposing the nested interactive if it receives focus)

scottaohara avatar Mar 17 '22 17:03 scottaohara

The Open UI Community Group just discussed Panelset/Spicy-Sections issue(s) review/rfc.

The full IRC log of that discussion <hdv> Topic: Panelset/Spicy-Sections issue(s) review/rfc
<hdv> github: https://github.com/openui/open-ui/issues/489
<hdv> bkardell_: so the group of us who is working on tabs and affordance have been wanting to take the work to Open UI and get it in a state where we can feel good about it
<hdv> bkardell_: previously `spicy-sections` was all in your light DOM
<hdv> bkardell_: that was less than ideal. For this, JonathanNeal did a lot of work and rewrites, There are some differences and we wanted to get some feedback today on if we're all okay with that
<bkardell_> https://tabvengers.github.io/spicy-sections/demonstration/
<hdv> bkardell_: so in here, there is a lot more Shadow DOM, everything is just as styleable
<hdv> bkardell_: in that link if you follow the Explainer link there is some more explanation
<hdv> bkardell_: under Parts you can find a breakdown of the parts, and information on how you can access all of the different parts and styling them
<hdv> bkardell_: there is really nothing hidden in the way that inputs have failed to be styleable, everything is exposed and styleable, but it is not all mucking about in your light DOM
<JonathanNeal> That “explainer” is a work in progress. The variants are printed out but not document.
<JonathanNeal> not documented.
<hdv> bkardell_: we have added a property called @@@, it can even be used in container queries
<hdv> bkardell_: there is no attribute sprouting here
<hdv> bkardell_: there has been some discussion, one of them has been about accessibility, we had good discussion in the GitHub and have resolved that. I don't foresee more accessibility issues, but would like to hear more feedback from the group
<hdv> bkardell_: this requires us to create sort of wrapper Shadow DOM to be used in the light DOM
<hdv> bkardell_: maybe Mason or Rob can talk about whether that would be problematic in implementation
<hdv> bkardell_: so, we would like to collect feedback and give it a week or two, to prepare something to transfer over to Open UI
<masonf> q+
<hdv> bkardell_: would like to know if this is the path we should pursue or not
<hdv> q?
<hdv> masonf: I wanted to quickly respond to the Shadow DOM question… from what you said it seems like the right way to use Shadow DOM. It sounds reasonable
<hdv> bkardell_: so the question is… we have to mint new Shadow DOM for every light DOM child
<hdv> bkardell_: and make sure everything gets slotted in
<hdv> masonf: so you look at light DOM children and create an appropriate amount of slots and then slot them in?
<hdv> bkardell_: that is what is happening yes
<hdv> masonf: sounds reasonable to me
<hdv> masonf: I don't know how you could have a fixed amount of slots, from your description it sounds reasonable
<hdv> bkardell_: are there arny precedents of Shadow DOM where we had to do that internally? because I assume you have to do bookkeeping, say the light DOM moves from first to second position, your Shadow DOM would need to adopt to that
<hdv> masonf: one example might be summary/details, they are implemented as slots in Shadow DOM, but that's a fixed set of two slots
<hdv> bkardell_: it seems better than the older way for the user, if we can do it, but I don't know if implementors would say we better don't
<hdv> masonf: what's most important is that it is easy to use from a developer point of view
<miriam> q+
<hdv> JonathanNeal: I'll add that the way it works on a high level… it looks at all of the child nodes, it begins almost like, misusing the phrase, garbage collecting… then when it finds, say, an HTML heading, it begins creating its first section of content, then everything after that, a section gets assigned for that panel
<hdv> JonathanNeal: in Safari manual assignment is not possible, so they don't get the same experience, but as soon as they do, their light DOM will also look untouched
<hdv> JonathanNeal: a workaround that may need to be addressed: for reasons like buttons that contain buttons, a heading that is inside a button, is semantically a button
<hdv> q+
<hdv> JonathanNeal: the third thing Brian pointed out is CSS… in the earlier version there was some CSS where you specified which affordance you wanted at a given media query. In this new version it is set in a property value, through the cascade or otherwise
<JonathanNeal> `--affordance: tab-bars;`
<hdv> bkardell_: to be extra clear about this… that was called out in every presentation we did… it is handwavy, that is not the proposal, only that it is in CSS or a CSS like language, eg you could change with your styling
<hdv> bkardell_: so this is a lot closer to what we would actually propose. But it is not clear yet that this is the property we would propsose, but closer to it. The toggle proposal and focusgroup proposal may play into this too
<masonf> FYI, I need to drop, sorry! I should have mentioned that earlier. But other folks in this meeting are quite familiar with the implementation. :-)
<hdv> bkardell_: what we had before was wonky in a particular way, on purpose
<hdv> bkardell_: a third question… to achieve this we are monitoring the value of the CSS property in a request animation frame
<hdv> bkardell_: that should not cost recalc, they are effectively getters… we did some testing and it does not seem to be a perf issue, but this is a question to folks
<hdv> ack masonf
<hdv> ack m
<scotto> q+
<hdv> miriam: the changes here are great, a real improvement
<hdv> q-
<hdv> miriam: I still have a few questions on the CSS, that is TBD, one thing I am interested in is… can we actually use the ::marker pseudo el
<hdv> JonathanNeal: thanks for the reminder that the current 'fake' marker is a part that needs to be documented
<hdv> ack scotto
<hdv> scotto: the only thing that jumps out of me, I'm curious what sort of descendant restrictions need to be applied here, eg a heading that contains a link
<hdv> scotto: as that would not work when it is a tab or expand/collapse
<hdv> scotto: this is also related to structured content nested in interactive… Firefox and Chromium mitigate for that
<bkardell_> q+
<hdv> scotto: one other bit I noticed when looking at this… arguably people seem to think that accordions need to have headings that surround buttons, this is the reverse… it looks like that may be off the table?
<hdv> scotto: another question is why the tabpanel would not be in the focus order, as generally people might want to escape the tabs with the tab key. In these cases there is no focusable content inside the panel so in this case someone using a screenreader would need to navigate backwards
<hdv> ack bkardell_
<hdv> bkardell_: some of the things you said might have been true in either approach, these are things we should probably log an issue about. We have tests to make sure we don't break things we already agreed on, so it is possible that there are some simple breakages in here
<hdv> bkardell_: I noticed also the tab content one, we will fix those when we fix the tests
<hdv> scotto: could you ping me when updates are done, I can give it another review

css-meeting-bot avatar Mar 17 '22 18:03 css-meeting-bot

There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label.

github-actions[bot] avatar Sep 26 '22 00:09 github-actions[bot]