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

[SELECT] The use of "part" clashes with custom elements containing <selectmenu> #354

Closed
dandclark opened this issue Jun 4, 2021 · 6 comments
Labels
select These are issues that relate to the select component

Comments

@dandclark
Copy link
Collaborator

Copy/pasted from MicrosoftEdge/MSEdgeExplainers#483 so we can discuss/bikeshed names in OpenUI. @mfreed7 says:

According to the explainer, here, when authors "replace" a part of the <selectmenu> using the slot attribute, they are required to also add the "part" attribute to allow the UA to connect up controller code. However, if a <selectmenu> is contained inside a custom element shadow root, that use of "part" will also expose the part for styling outside the custom element, since the "part" syntax is identical and there is no direct-child requirement for using "part".

Here is an example:

<my-custom-select>
  <template shadowroot=open>
    <selectmenu>
      <div slot="button" part="button">Custom button</div>
      <option>Cat</option>
      <option>Dog</option>
    </select>
  </template>
</my-custom-select>

<style>
  my-custom-select::part(button) {
    /*This will match the button inside the custom element*/
    background-color: red;
    padding:20px;
  }
</style>

Note that the replaced custom "button" part will be exposed outside the <my-custom-select> element.

Perhaps we could either use a different attribute name ("custom-part", "replaced-part", ...?) or change the behavior/spec for CSS custom parts so that it somehow magically "hides" these contained parts?

Thanks to @justinfagnani for pointing this out.

@dandclark
Copy link
Collaborator Author

@justinfagnani asked:

@dandclark is there a place in the explainer that says why slot alone is insufficient?

I'm not sure that it's specifically laid out in the explainer; probably I should add something on this.

Having part (or whatever we name it) as a separate attribute grants a lot more power to authors who want to be selective about where the native controller code is applied. The example in Another example of extensibility: Split-button select, modeled after Bootstrap's Splitbutton, could not be written as easily without this capability. The author wants to replace the entire default button of the <select>, so they use slot="button" to replace it with their own custom-styled content. But, they only want the right-hand section of their button to get the native event handlers that open the listbox on click/keyboard, so they apply part="button" to only the <div class="arrow" part="button"></div>.

<select>
  <div slot="button-container">
    <div class="select-label">Primary</div>
    <div class="arrow" part="button"></div>
  </div>
  <div name="listbox-container" slot=”listbox-container” part=”listbox”>
    <option value="Option One">Option One<option>
    <option value="Option Two">Option Two</option>
    <option value="Option Three">Option Three</option>
    <div class="separator"></div>
    <option value="Option Four">Option Four</option>
  </div>
 </select>

@dandclark
Copy link
Collaborator Author

As I said in the original thread, I'd previously thought about the dual-use of part as a potential advantage, in cases where custom control authors might want to have their custom parts be styleable by the outer page. But the inability to prevent the styleability from leaking seems bad. I think the simplest solution is, as proposed, just use another name for the attribute.

@mfreed7 said:

I kind of prefer a longer name here - I think it will be important to distinguish this from the existing Shadow Parts API. Perhaps we could use a word that doesn't even include "part"?

Brainstorming some possibilities:

  • subcomponent
  • subcomponentname
  • piece
  • controlsegment
  • hasbehavior, or just behavior, like behavior="button"

Or if we're OK with allowing "part" in the name:

  • controlpart
  • componentpart
  • controlsubpart
  • etc

Naming is hard...

@dandclark
Copy link
Collaborator Author

In #396 (comment) @chrisdholt suggested using role instead of part for applying controller code. I like this idea because:

  • Semantically the word role seems to work.
  • If the author is required to specify the correct value of role in order to have controller code applied to an element, then there is less magic that the controller code needs to do to assign the correct accessibility role to that element.

However there are two problems to this that I can't think of a way around.
The first is that I think it would cause problems with the splittbutton example I wrote about in #354 (comment).

<select>
  <div slot="button-container">
    <!-- We don't want to give this one controller code, since it shouldn't open the listbox,
            but it needs the role attr to have the correct a11y semantics: -->
    <div class="select-label" role="button">Primary</div>
   <!-- We do want to give this one controller code so that it opens the listbox:-->
    <div class="arrow" role="button"></div>
  </div>
  <div name="listbox-container" slot=”listbox-container” role=”listbox”>
    <option value="Option One">Option One<option>
    <option value="Option Two">Option Two</option>
    <option value="Option Three">Option Three</option>
    <div class="separator"></div>
    <option value="Option Four">Option Four</option>
  </div>
 </select>

There's no way to distinguish between an element given a role because the author wanted it to receive <selectmenu> controller code, and an element given a role because the author just wanted to give it the correct a11y semantics.

Secondly, there are UI parts that might not have a corresponding role, e.g. the "selected-value" part in https://open-ui.org/components/select#anatomy-1. "selected-value" is probably not critical to include and could maybe be dropped from the anatomy, but there are other cases of this that probably can't be dropped. For example, checkbox has an "indicator" part that doesn't really correspond to an ARIA role.

So I can't think of a way around these problems, but the use of role for applying controller code is an interesting enough idea that I wanted to raise it here in case anyone has ideas for making it work.

@chrisdholt
Copy link
Collaborator

There's no way to distinguish between an element given a role because the author wanted it to receive <selectmenu> controller code, and an element given a role because the author just wanted to give it the correct a11y semantics.

Secondly, there are UI parts that might not have a corresponding role, e.g. the "selected-value" part in https://open-ui.org/components/select#anatomy-1. "selected-value" is probably not critical to include and could maybe be dropped from the anatomy, but there are other cases of this that probably can't be dropped. For example, checkbox has an "indicator" part that doesn't really correspond to an ARIA role.

On this - I think it really comes down to what we want to enable as the platform and what is the responsibility of an implementor. One of the primary reasons that I elevated role is that it ties the functional code to accurate a11y semantics. As you point out, this does create a problem in scenarios where you want something to get controller code but isn't necessarily accurate from an accessibility standpoint. I think in some sense, we need to pick a direction here in terms of which way we bend. Do we prioritize tying the controller code functionally to an attribute like role which enables an accurate representation for all users, or do we prioritize flexibility to support select doing anything someone wants, which may not be able to be made accessible regardless. My bent in this case is always to lean into prioritizing accessibility here, but that is my opinion. I could also see where that creates inaccessible controls because people do wrong things. On the other hand, if we prioritize something like part, then ensuring an accurate role is again up to the implementor...so that's somewhat a wash.

@dandclark, on the note regarding checkbox...what portion of the controller code is tied to the actual indicator part? I think an indicator would be slotted, but I can't necessarily think of a reason that we would need to tie behaviors to the visual indicator itself. Elements with a role of checkbox or radio will both end up labelling their children as presentational so anything in this slot is likely not interactive or accessible from an AT perspective.

@css-meeting-bot
Copy link

The Open UI Community Group just discussed [SELECT] The use of "part" clashes with custom elements containing.

The full IRC log of that discussion <gregwhitworth> Topic: [SELECT] The use of "part" clashes with custom elements containing
<hdv> github: https://github.com//issues/354
<hdv> dandclark: we had some name bikeshedding with this one
<hdv> dandclark: this is what selectmenu will look for… it overlaps with the CSS feature that allows them to be styled outside of the Shadow DOM
<hdv> dandclark: this would be a problem if authors would want to style selectmenus outside Shadow DOM
<hdv> dandclark: see the example in the first comment on the issue https://github.com//issues/354#issuecomment-855003752
<hdv> dandclark: we thought of this as a feature, but there may be times where authors do not want this
<hdv> dandclark: so basically, we probably need a different attribute name, that is not 'part'
<hdv> dandclark: I had some ideas… component, subcomponents, segments, behaviour, controlpart, componentpart, controlsubpart…
<gregwhitworth> q?
<hdv> dandclark: not married to any of these, if I had to pick one today, probably like controlpart
<hdv> masonf: might be nice to include the fact that it applies controller code, so maybe controlledpart? is quite long though
<hdv> gregwhitworth: *asks for clarfication*
<hdv> dandclark: so what part="button" does is that this will become the button and get the button behaviour, so that's desirable… but we have also used it in another meaning… if I have some selectmenu in the Shadow DOM of my custom element… 'part' is also used in CSS to pierce the Shadow root
<hdv> dandclark: so it is used both for selectmenu controller code behaviour and for this CSS feature to expose the element outside the Shadow DOM
<hdv> dandclark: those are both desirable at different times but not at the same time
<hdv> gregwhitworth: when we first talked about this… we talked about updating the parts specification
<hdv> dandclark: we came up with cases where it would be useful to have it
<hdv> dandclark: we found there are cases where that might be useful, but also cases where it was not avoidable
<hdv> dandclark: but maybe in this case people might not want this part to be styleable outside their custom elements, and in this case that might be a problem
<hdv> gregwhitworth: I don't really want to add a new content attribute just for this, if we're not absolutely sure if it is a developer painpoint
<hdv> nicole: it is
<hdv> nicole: what I heard from Stencil, they liked to encapsulate CSS and the DOM bit, but it was not their expectation that those would go together, they had use cases for either separately
<hdv> nicole: maybe CSS is a better place for that to live
<hdv> gregwhitworth: I feel like you've just confirmed people want this
<hdv> nicole: yes, that is what I;'ve heard from web developers
<hdv> gregwhitworth: and scoping would not solve this?
<hdv> nicole: it would not solve it for web components… 
<hdv> masonf: there is an open issue for open styleable
<hdv> masonf: but for the folks that do want scoped styles the way they currently work… that's already defined, with the part attribute
<hdv> masonf: maybe these are features that don't overlap 100% and therefore should not be shared?
<gregwhitworth> q?
<hdv> gregwhitworth: I don't think it can be controlparts, because we might get into that 'what is a control' again discussion
<hdv> gregwhitworth: I kind of like behavior
<masonf> +1 I also like `behavior=button`, that feels natural.
<hdv> gregwhitworth: a long time ago, someone wanted to stack these up, like here are the behaviors I want to apply to this
<hdv> gregwhitworth: behavior feels to me like it is about controller code
<hdv> davidluhr: one thing that came to mind in this discussion… the pattern that comes up in React, the as attribute… so maybe `as=part`
<hdv> davidluhr: to say, this will become the button part
<hdv> gregwhitworth: so the example would have been `as="button"`… or maybe `as-part="button"` to say it would render out as part of the component
<hdv> s/or maybe `as-part="button"` to say it would render out as part of the component//
<hdv> davidluhr: or maybe `as-part="button"` to say it would render out as part of the component
<hdv> masonf: it's kind of hard to disambiguate the two types of parts, so having `behavior` is a nice way to do that
<hdv> gregwhitworth: the more we talk about tit the more I like it
<flackr> +1 to behavior
<hdv> gregwhitworth: I feel `as` is not complete clear to me… davidluhr would you be opposed to behavior?
<hdv> davidluhr: not at all
<gregwhitworth> q?
<scottohara> +1 to behavior
<hdv> BoCupp: I'm late but are we absolutely sure that we don't want to couple it to the part attribute?
<hdv> gregwhitworth: yes you missed me ask that, nicole said the Ionic team have scenarios where they want to treat them separately
<hdv> nicole: I was surprised too
<hdv> nicole: it was a couple of years ago, maybe worth going back to there
<hdv> flackr: overloading part to have two things that it does that are kind of related is bad design, we should try to avoid it
<hdv> flackr: someone said they're not the same use cases… I think it would be bad
<hdv> gregwhitworth: objection to CSSWG not naming this correctly :-)
<hdv> gregwhitworth: maybe good, nicole, to reach back out to them to see if they still have that use case
<masonf> Need to run to another meeting, sorry. Sounds like this is on a good track though...
<hdv> gregwhitworth: also we can always resolve resolutions
<BoCupp> no objection
<dandclark> Proposed resolution: use "behavior" content attribute instead of "part" content attribute for applying controller code
<hdv> gregwhitworth: any objections?
<hdv> Resolved: use "behavior" content attribute instead of "part" content attribute for applying controller code
<hdv> dandclark: a meta question… when writing spec text, is part still what we want to call it in spec text? we can call it whatever, but is that the right way to refer to it?
<hdv> gregwhitworth: kind of, it is still a part of the component
<hdv> gregwhitworth: thanks everyone,!
<hdv> RRSAgent, make minutes please
<RRSAgent> I have made the request to generate https://www.w3.org/2021/10/28-openui-minutes.html hdv
<gregwhitworth> Zakim, end meeting
<Zakim> As of this point the attendees have been hdv, dandclark, flackr, masonf, una, miriam, iopopesc, nicole, BoCupp, scottohara
<Zakim> RRSAgent, please draft minutes
<RRSAgent> I have made the request to generate https://www.w3.org/2021/10/28-openui-minutes.html Zakim
<Zakim> I am happy to have been of service, gregwhitworth; please remember to excuse RRSAgent. Goodbye

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 2, 2021
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 2, 2021
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 3, 2021
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 4, 2021
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258284
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#938285}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 4, 2021
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258284
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#938285}
pull bot pushed a commit to Mu-L/chromium that referenced this issue Nov 4, 2021
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258284
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#938285}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 10, 2021
…ute instead of part for applying controller code, a=testonly

Automatic update from web-platform-tests
[SelectMenu] Use behavior content attribute instead of part for applying controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258284
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#938285}

--

wpt-commits: 9d42cfbd911a281dd0b83eb9045ff7cfb6622ba9
wpt-pr: 31479
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 10, 2021
…ute instead of part for applying controller code, a=testonly

Automatic update from web-platform-tests
[SelectMenu] Use behavior content attribute instead of part for applying controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258284
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#938285}

--

wpt-commits: 9d42cfbd911a281dd0b83eb9045ff7cfb6622ba9
wpt-pr: 31479
@dandclark
Copy link
Collaborator Author

Per the resolution here to use "behavior" instead of "part", I think we can consider this to be resolved. Closing.

Gabisampaio pushed a commit to Gabisampaio/wpt that referenced this issue Nov 18, 2021
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258284
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#938285}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
…ing controller code

Per OpenUI resolution in: openui/open-ui#354,
the "behavior" content attribute is used for applying controller code,
while "part" is going to be only used for exposing the part for
styling.

Bug: 1121840
Change-Id: Icc0010addab58bbb810f772f46318e1b8fcc5e64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3258284
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Dan Clark <daniec@microsoft.com>
Commit-Queue: Ionel Popescu <iopopesc@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#938285}
NOKEYCHECK=True
GitOrigin-RevId: c8ce327437bd7897b2321a985e160c04391e07d0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
select These are issues that relate to the select component
Projects
None yet
Development

No branches or pull requests

3 participants