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

[selectmenu] Accept "option" elements through multiple slots #565

Closed
Westbrook opened this issue Jul 9, 2022 · 15 comments
Closed

[selectmenu] Accept "option" elements through multiple slots #565

Westbrook opened this issue Jul 9, 2022 · 15 comments
Labels
agenda+ Use this label if you'd like the topic to be added to the meeting agenda needs edits This is ready for edits to be made select These are issues that relate to the select component

Comments

@Westbrook
Copy link

Referencing the awesome demos for this element available at https://microsoftedge.github.io/Demos/selectmenu/ I am thinking about how a developer might like to components the delivery of such a customization of the <selectmenu> element. This will likely be of high importance to design system developers who will need to ensure the newly possibly custom styling of this element is normalized across a broad field of usage.

For instance, working from the Dribble Filter example (for its simplicity, but the ideas below could easily apply to any of the demos listed), it would be great to be able to make a custom element to encapsulate the design delivery, while reducing the lift a consuming developer needs to recreate the visual and functional delivery via something like:

<x-selectmenu>
  <option value="now" checked="">Now</option>
  <option value="week">This Past Week</option>
  <option value="month">This Past Month</option>
  <option value="year">This Past Year</option>
  <option value="all">All Time</option>
  <template shadowroot="open">
    <selectmenu id="timeframe-filter">
          <div slot="button" behavior="button">
            <button behavior="selected-value">This Past Year</button>
          </div>
          <slot></slot>
        </selectmenu>
    <style>
      button {
        padding: .5rem;
        border-radius: .3rem;
        width: 100%;
        text-align: left;
        box-shadow: 0px 0px 0px 1px #e7e7e9 inset;
        border: 0;
        background: white;
        color: #6e6d7a;
        font-size: 1rem;
        display: flex;
        align-items: center;
        justify-content: space-between;
        gap: 1rem;
        cursor: pointer;
      }
      
      button:active {
        transform: translate(1px, 1px);
      }
      
      button:hover,
      selectmenu:focus-within button {
        color: black;
      }
      
      button:hover,
      selectmenu:focus-within button:not(:active) {
        box-shadow: 0 0 0 4px rgba(234, 76, 137, 0.1);
      }
      
      button::after {
        content: '';
        width: 1rem;
        height: 1rem;
        background: #e7e7e9;
        clip-path: polygon(10% 20%, 90% 20%, 50% 80%);
        transform-origin: center;
        transition: transform .2s ease-in-out;
      }
      
      selectmenu:focus-within button::after {
        transform: scaleY(-1);
      }
      
      [popup] {
        border-radius: .3rem;
        border: none;
        box-shadow: 0 0 .3rem rgba(0, 0, 0, .25);
        padding: 0;
        margin-top: .5rem;
      }
      
      ::slotted(option) {
        padding: .5rem;
        color: #6e6d7a;
        cursor: pointer;
        padding-right: 2rem;
        display: flex;
        gap: .5rem;
        align-items: center;
      }
      
      ::slotted(option:hover) {
        background: #e7e7e9;
      }
      
      ::slotted(option:checked) {
        color: #ea4c89;
      }
    </style>
  </template>
</x-selectmenu>

This way a consuming developer really only need to worry with:

<x-selectmenu>
  <option value="now" checked="">Now</option>
  <option value="week">This Past Week</option>
  <option value="month">This Past Month</option>
  <option value="year">This Past Year</option>
  <option value="all">All Time</option>
</x-selectmenu>

Does it look like something we can expect the specification to cover?

In particular, the <option> elements are being passed through a <slot> into a deeper <slot> that lists the in the <selectmenu> as its options.

Neither of the current Chrome and Edge Canary releases seem to support this via this Code Pen, but hopefully this is just early days still and the move to open up the broader styling and usability of the styling on <select> style interfaces includes this capability.

@dandclark
Copy link
Collaborator

I expect the specification to cover this use case. The <selectmenu> should find its options using a "flat tree" walk, which is basically the view that you get after all the slotting has been resolved.

I haven't had a chance to dig into it yet but I suspect this is just a bug in the implementation where the <selectmenu> isn't finding the doubly-slotted <options>, or perhaps some issue with routing events across shadow DOMs.

I'm spread pretty thin right now but as soon as I can I'll take a look and see if I can provide a fix.

@Westbrook
Copy link
Author

That's awesome. Thanks @dandclark!

@mfreed7 mfreed7 added the select These are issues that relate to the select component label Aug 15, 2022
@josepharhar
Copy link
Collaborator

In the codepen, I can see the options being rendered in the selectmenu's dropdown, but clicking on them does not appear to change the selected option. Is that the bug?

@josepharhar
Copy link
Collaborator

I don't think this is an easy to support use case.

The selectmenu implementation uses child/descendant changes as a hook to look for new option elements, in addition to custom listbox/button/selected-value parts. If we were to allow other elements in the document to be slotted in and considered options, we would have to check for new option elements in the flat tree every time any node in the document is modified - something that I don't think we do for any other elements and would probably not be very performant.

One of the HTML editors is also against using the flat tree at all in HTML, so this would likely be hard to spec as well: whatwg/html#8970

I would like to support this use case for custom elements if possible, but if we can't you can still do pretty much everything that selectmenu does through custom elements by using form-associated custom elements, declarative/imperative slotting, and the popover attribute.

I propose that we don't support this use case, but I'd like to see what others have to say so I'm going to agenda+ this issue

@josepharhar josepharhar added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Mar 24, 2023
@Westbrook
Copy link
Author

you can still do pretty much everything that selectmenu does through custom elements by using form-associated custom elements, declarative/imperative slotting, and the popover attribute.

Feels like if this was fully true that we’d never have needed <selectmenu> in the first place.

With all of the content that can be slotted here, it makes sense that there would be a lot of complexity here. If the answer was really to lean towards “no” here, is it crazy to investigate partial support here? No idea if it still hits the same issues, but could slotting <option>s directly into the <selectmenu> be acceptable, but not when slotting an options list? It feels like this is going to be a pretty widely asked for feature from my fellow developers, so a middle ground more so than a nothing here would be amazing!

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 25, 2023
This is being discussed here:
openui/open-ui#565

Bug: 1121840
Change-Id: I9eb659142bfaa876656613ee97f33adacdf13f67
@josepharhar
Copy link
Collaborator

Upon further investigation, it looks like the selectmenu implementation in chromium is already watching for all DOM changes in the document anyway when looking for child options and button/selected-value/listbox parts, so I tweaked it to look at the flat tree for options as well. This patch makes your codepen work: https://chromium-review.googlesource.com/c/chromium/src/+/4371978

At first I thought that we should have this slotting/flat-tree behavior for button/selected-value/listbox parts which would make it more complicated, but I realized that won't work properly since they must be slotted directly into the the selectmenu with specific slot attributes, which won't work with nested slotting. @Westbrook do you want to be able to nestedly slot custom buttons and listboxes for this use case, or is just options and anything else that would be put into the default slot in the listbox good enough?

I still think this will be hard to spec and I can't make any guarantees that it will survive the HTML spec review, but I suppose that I am now supportive and propose that we support this use case.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 25, 2023
This is being discussed here:
openui/open-ui#565

Bug: 1121840
Change-Id: I9eb659142bfaa876656613ee97f33adacdf13f67
@Westbrook
Copy link
Author

Thats great news!!

As a maximalist, looking to see if there are ways to make everything "just work™️", would be more than amazing. There are a number of interesting slotting practices that could be leveraged here to interesting ends. I'd be hard press to whip up some practical use cases without some time to spend specifically investigating the pattern, however. In the end, as I mentioned above, some in better than none. If we can only get elements in the default slot, I'd call that a win.

Thanks for reporting back with your findings 🙇🏼

@css-meeting-bot
Copy link

The Open UI Community Group just discussed [selectmenu] Accept "option" elements through multiple slots #565.

The full IRC log of that discussion <dandclark> Topic: [selectmenu] Accept "option" elements through multiple slots #565
<hdv> github: https://github.com//issues/565
<dandclark> jarhar: There was feature request to be able to slot in options from outside the selectmenu itself
<dandclark> ...: Have those slotted again into listbox
<dandclark> ...: lets say you've got a custom element that has a selectmenu
<dandclark> ...: It puts a default slot element inside the selectmenu. Someone using the custom element gives a bunch of option elements as child nodes in light dom to custom element.
<dandclark> ...: That custom element slots 'em into the selectmenu, which slots them into its listbox
<dandclark> ...: hesitant to support at first, since seems complicated
<dandclark> ...: And then there's issue of whether to support for other parts
<dandclark> ...: but was able to get nested slotting for listbox working in Chrome
<dandclark> ...: Don't think it works for other parts since they need a slot attribute on them
<hdv> q?
<dandclark> ...: And then user of custom element needs to put slot element on thing that wants to be the button, and then slotting it again into the selectmenu ???
<dandclark> ...: Propose we support nested slotting for listbox but not the other parts.
<bkardell_> q+
<hdv> ack bkardell_
<dandclark> bkardell: I looked at this, you can see what he's doing in it. At some level it feels like not that big a stretch to imagine it's useful to do something like that. Can provide more than a selectmenu kind of control. Want people to provide the options.
<dandclark> ...: feels still very complicated
<dandclark> ...: I don't have strong feelings other than it wil be interesting to see if we can get the a11y to be not easy to blow up
<dandclark> JakeA: FWIW I don't feel able to get into this level of detail before there's an explainer
<dandclark> jarhar: Resolve to suport this use case until other problems come up?
<dandclark> hidde: Should we get a wider net to see if there's more people with opinions
<dandclark> JakeA: Will struggle with that without an explainer
<dandclark> hidde: Resolve to make explainer? :)
<dandclark> jarhar: There is one that describes slotting
<dandclark> JakeA: Was told it's out of date
<jarhar> https://open-ui.org/components/selectmenu/
<dandclark> dandclark: I think that one is still fairly up to date
<JakeA> I gotta bail, sorry!
<dandclark> hidde: SHould we leave issue open for a bit longer to see what people want to say? Otherwise resolution is based on too few people
<dandclark> jarhar: I have it implemented in Chrome, can merge patch whenever we resolve

@dandclark
Copy link
Collaborator

I was too busy trying to scribe to think through this through during the meeting, but looking at it again now I still think the use case makes sense and it was basically an implementation bug that this didn't work in the Chromium protototype.

When previously discussed (e.g. #291) the behavior we've converged on is that parts like options get picked up by the selectmenu if they are a flat-tree child of the selectmenu. In @Westbrook's example, the doubly-slotted option ends up as a flat-tree child of the selectmenu once all the slotting resolves, so it should just work.

The Chromium implementation tries to be clever about how we look for parts so that we don't need to do unnecessary tree walks under the selectmenu for every DOM mutation. And it seems in doing so we made a mistake such that this example doesn't work. But if we can fix that bug then I'm not sure this issue even needs a meeting resolution unless it's to depart from the previously-decided behavior of using the flat tree to find selectmenu parts.

@gregwhitworth gregwhitworth added needs edits This is ready for edits to be made and removed agenda+ Use this label if you'd like the topic to be added to the meeting agenda labels Apr 5, 2023
@josepharhar josepharhar added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Apr 5, 2023
@josepharhar
Copy link
Collaborator

Here is a minimal example of the use case we're trying to support:

<div>
  <template shadowrootmode=open>
    <selectmenu>
      <slot></slot>
    </selectmenu>
  </template>
  <option>one</option>
  <option>two</option>
</div>

@css-meeting-bot
Copy link

The Open UI Community Group just discussed [selectmenu] Accept "option" elements through multiple slots, and agreed to the following:

  • RESOLVED: Support nested slotting of option elements. Selectmenu should look at the flat tree for its parts. For corner cases such as encountering another selectmenu in the flat tree, skip over it and don't look into it for more parts.
The full IRC log of that discussion <gregwhitworth> Topic: [selectmenu] Accept "option" elements through multiple slots
<gregwhitworth> github: https://github.com//issues/565
<gregwhitworth> jarhar: this usecase is about doing nested slotting to get nested option elements into the selectmenu
<gregwhitworth> there is a default named slot that are placed into the listbox in the shadow root
<dandclark> q+
<gregwhitworth> jarhar: what if I put the entire selectmenu into my own shadow and then I try to slot them into that one?
<gregwhitworth> jarhar: the current impl doesn't work this way and I was concerned about impl and more importantly spec'ing it but it was pretty easy to impl
<jarhar> https://github.com//issues/565#issuecomment-1499179800
<gregwhitworth> ^ example
<gregwhitworth> jarhar: I think we should support it and dandclark noted that we partially resolved on this in another issue
<gregwhitworth> ack dandclark
<gregwhitworth> dandclark: it was kind of scary last meeting but it's not a complicated case and it distills this. Early on we discussed this for parts to hook up its magic, it does a flat tree walk
<gregwhitworth> dandclark: in its shadow root and if we assume that behavior this should just work because the options that get slotted into the light DOM then that should be in the flat tree walk, we don't need to invent something new
<gregwhitworth> dandclark: in the scenario that it doesn't work in Chromium is because we tried to be clever
<gregwhitworth> dandclark: walk the flat tree and pick up parts as you go and hook things up again so we tried for a perf optimization and we messed something up here
<gregwhitworth> dandclark: I noticed that jarhar had a CL that potentially fixes it, I think we should resolve and not invent anything new here
<gregwhitworth> q?
<gregwhitworth> xiaochengh: we're going to select all the flat tree menu of the selectmenu children
<gregwhitworth> xiaochengh: what if the children of a selectmenu, do we want to add some future proofing for boundary elements?
<gregwhitworth> dandclark: I remember in the Chromium impl, you stop when you hit another selectmenu to not reach into it and slot further down than would have expected
<gregwhitworth> dandclark: there was an issue about a lot of corner cases with duplicating parts, buttons inside of options, etc. I feel like we did discuss that but we don't have the issue at hand
<bkardell_> present +
<gregwhitworth> dandclark: one thing we can do is to affirm that we do a flat tree walk but for corner cases we'll define or articulate current resolutions if they aren't handled well
<jarhar> Proposed resolution: Support nested slotting of option elements. Selectmenu should look at the flat tree for its parts. For corner cases such as encountering another selectmenu in the flat tree, skip over it and don't look into it for more parts.
<jarhar> RESOLVED: Support nested slotting of option elements. Selectmenu should look at the flat tree for its parts. For corner cases such as encountering another selectmenu in the flat tree, skip over it and don't look into it for more parts.

@xiaochengh
Copy link

Wait, should we do this for options only, or for any part?

@josepharhar
Copy link
Collaborator

I thought that slotting in other parts might not be a thing you can do with shadowdom, but lets see if this example works:

<div id=host1>
  <template shadowrootmode=open>
    <div id=host2>
      <template shadowrootmode=open>
        <slot name=two></slot>
      </template>
    </div>
    <slot name=one slot=two></slot>
  </template>
  <div slot=one>doubly named slotted element</div>
</div>

It looks like this doesn't work in any browser, so I don't think that doing this for other parts is currently possible due to limitations of ShadowDOM itself.
If this is something that we must have then we could try to change ShadowDOM to support this, but for now I think we can just not support it - at least until ShadowDOM supports slotting a slot element into a named slot.

@xiaochengh
Copy link

We can still do it with some wrapper elements:

<div id=host1>
  <template shadowrootmode=open>
    <selectmenu>
      <div slot=listbox>
        <slot name=external-listbox></slot>
      </div>
    </selectmenu>
  </template>
  <div popover behavior=listbox slot=external-listbox>
    <option>A</option>
    <option>B</option>
  </div>
</div>

(Not sure why we can't directly slot a <slot> into a named slot though... Either there's a browser bug or I misread the spec, but it doesn't matter here)

@josepharhar
Copy link
Collaborator

Ah, I didn't think of that, thanks for making that example. I think that Dan felt strongly that we should just use the flat tree to support everything that isn't totally bizarre, so we should probably support this case. I'll see how hard it is to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda+ Use this label if you'd like the topic to be added to the meeting agenda needs edits This is ready for edits to be made select These are issues that relate to the select component
Projects
None yet
Development

No branches or pull requests

7 participants