Users/procload/add select spec

[procload]

Select spec for web component

[fabricteam]

📊 Bundle size report

_{🤖 This report was generated against bdb9124866e9a0c64b5a609826a9178c18e7e1ad}

[size-auditor]

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: bdb9124866e9a0c64b5a609826a9178c18e7e1ad (build)

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 46bf6d3:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

[miroslavstastny]

lgtm

[brianchristopherbrady]

approved

[chrisdholt]

aria-label will work here, but technically until we get cross root ARIA support the other two won't map to external labels. In terms of associating with a label, you probably need it in the shadow root in this instance (technically)

[miroslavstastny]

So removing labelledby and describedby or keeping it?

[chrisdholt]

Are form and size necessary to pass here as well? With that in mind, what does form association look like for this given the shadow DOM? I have a feeling we'll need to recreate any events that are natively emitted from the select.

[procload]

I believe size is necessary as the Fluent React select has a size attribute but it doesn't align to the native size element from the browser, which corresponds to the number of visible rows when using the multiple attribute. But since we're not using multiple, can we keep size as an attribute that reflects the same intention as the Fluent React version?

[chrisdholt]

Well, size is different here in both cases. So, I don't think you can have conflicting attributes - one would need to be different. So, if size as it's detailed in the MDN docs is not supported because multiple is not, we should call that out. We should probably call out that multiple is also not suppoprted.

[procload]

@miroslavstastny Do we have any other recommendations or precedent for the word size if the size attribute means something else? component-size? height? Open to suggestions.

[chrisdholt]

Given multiple isn't supported here, this is probably primarily a problem for at least "Dropdown" and possibly "Combobox"

[procload]

Are you implying that it's acceptable to use size here as an attribute since we're not supporting multiple? I'm cool either way, just want to make sure we're on the same page here. A good practice might be to mimic whatever we're using for Dropdown here. I know we're using size elsewhere as a way to change the size, e.g. Slider.

[chrisdholt]

The opposite - size isn't needed here but the question for @miroslavstastny may be pertinent where multiple is suported.

[miroslavstastny]

I got lost in the discussion here, sorry.
FUIR9 supports size="small"|"medium"|"large" which affects font, height and spacing:

fluentui/packages/react-components/react-select/src/components/Select/useSelectStyles.ts

Lines 144 to 149 in 5745de0

	small: {
	height: fieldHeights.small,
	paddingLeft: paddingLeft.small,
	paddingRight: paddingRight.small,
	...typographyStyles.caption1,
	},

So we will need to support this kind of sizing in FUIWC3 as well, right?

I would expect the attribute name to be consistent across all the three components - select, dropdown and combobox. If we cannot use size in all three we need to find a new consistent name. I would propose control-size but not sure it is any better than component-size.

[chrisdholt]

Issue isn't in select, but for anything with multi-select, there is a size attribute which already exists in the native API for how many are shown in the listbox.

[chrisdholt]

I think we should consider a label slot until we get the cross root aria stuff figured out.

[procload]

If we have a label slot, does it make sense to use the wrap the slot in a native HTML label element, or should we use the fluent-label web component here?

[procload]

Also: how do we feel about adding a label slot for this specific component when the other components don't? Is it inconsistent? Is that a bad thing?

[chrisdholt]

Great question - what are the implications with a slot vs. without one?

[miroslavstastny]

native HTML label element, or should we use the fluent-label web component

We should always weight what % of the custom component will be used in the particular scenario (vs what will be added to the bundle) and also how difficult would it be to recreate the component in place.
And imo the biggest factor is - when we will change fluent-label, would you expect the label in fluent-select to change as well?
In this case, we will perhaps use most of the fluent-label functionality, the select's label should be consistent with fluent-label so it is ok to use fluent-label.

how do we feel about adding a label slot for this specific component when the other components don't? Is it inconsistent? Is that a bad thing?

Inconsistency is definitely a bad thing. But in this case, I think it is an temporary earned difference until we have cross root aria.

[chrisdholt]

I think you'll need to mimic the Select API here for setting/getting value as well as holding a reference to options, etc. Here's the broader API: https://developer.mozilla.org/en-US/docs/Web/API/HTMLSelectElement

[procload]

To make sure I'm tracking, would this look like:

get options() {
  return this.querySelectorAll('option');
}

get length() {
  return this.options.length;
}

get value() {
  const selectedOption = this.querySelector('option:checked');
  return selectedOption ? selectedOption.value : '';
}
...
etc

[chrisdholt]

I can't speak to the exact implementation here for now but I would avoid using querySelector in these ways. There are better methods at our disposal to hold a reference to these via observables + directives.

[chrisdholt]

But yes, we would need to implement the whole of the interface on the host element. Technically you could use a reference on the select and return whatever the select implementation is...

[procload]

Some quick Googling told me the following would work:

  @observable selectElement: HTMLSelectElement;

  get value(): string | null {
    return this.selectElement ? this.selectElement.value : null;
  }

  set value(newValue: string | null) {
    if (this.selectElement) {
      this.selectElement.value = newValue;
    }
  }

  get length(): number {
      if (this.selectElement) {
        return this.selectElement.length;
      } else {
        return 0;
      }
    }

And then my template would resemble:

<select ref=${(x) => (x.selectElement = x)}>
 Default Slot Content
</select>

Does this track to what you're thinking?

Apologies if this is too much back-and-forth for a Spec PR, but I'd like to get this nailed down before implementation.

[chrisdholt]

I think these would both go in the default slot FWIW - the larger question here is if we want to support both. Then we should likely also ask whether we want to filter for those elements to ensure that other passed elements don't get rendered within it.

<fluent-select>
  <option value="option1">Option 1</option>
  <option value="option2">Option 2</option>
  <optgroup label="Group 1">
    <option value="option3">Option 3</option>
    <option value="option4">Option 4</option>
  </optgroup>
</fluent-select>

[procload]

Would this be the first time we would filter out elements that don't align to the component? I can understand the reasoning why we want to do this, but given the relative lax guidelines in terms of letting developers perform non-standard operations on our components, do we want to lock this one down? It seems like it violates some of what we previously discussed about enforcing standards through linting vs. baked into the component.

[chrisdholt]

the relative lax guidelines in terms of letting developers perform non-standard operations on our components, do we want to lock this one down

Can you elaborate on this? When it comes to something like menu, only menuitem, menuitemradio, and menuitemcheckbox elements are included in the taborder for instance. Because we support divider and in the future "groups", we don't filter...but there absolutely are rigid requirements on behaviors across the components.

I'm not sure that I'd agree with the term "lax" in the context of linting vs. baked here. Any proposals I've made on linting is around design guidance where we want only subsets of appearances supported for certain components. The approach for FUIR9 has been to create "sub components" with only those variations supported. My recommendation is that runtime code being added to enforce design guidance is misled. I don't think that's the same as ensuring that only supported elements get projected into the slot of a component. The intent of those two things and the corresponding costs are different.

To be clear, I'm asking on filtering, not prescribing...though I think there is merit to it. What are the implications if we do vs. do not?

[procload]

After fooling around with this some more and putting some real code in place, I noticed the following behavior. I'm not sure if this is a deal breaker, but thought it might be worth discussing:

My template code looks like:

export function selectTemplate<T extends Select>(): ElementViewTemplate<T> {
  return html<T>`
    <template>
      <label part="label" for="control" ?hidden="${x => !x.label}"> ${x => x.label} </label>
      <select
        id="control"
        part="control"
        :autofocus="${x => x.autofocus}"
        :autocomplete="${x => x.autocomplete}"
        ?disabled="${x => x.disabled}"
        @change="${x => x.handleChange()}"
        @input="${x => x.handleInput()}"
      >
          <slot @slotchange="${x => x.handleSlotChange()}"></slot>
      </select>
    </template>
  `;
}

But when I try to pass the option values in when using it, they render outside of the select control in the Shadow DOM, ala:

I might be able to use JavaScript to move those in but that seems expensive? Is there a way around this?

[miroslavstastny]

To be clear, I'm asking on filtering, not prescribing...though I think there is merit to it. What are the implications if we do vs. do not?

For me the preferred approach would be to have a runtime error in development mode (and ideally a lint rule) and nothing in production.

1. we do not want to break in production
2. filtered out elements are hard to debug.

[procload]

@chrisdholt @miroslavstastny I pushed up the latest spec revisions based on our discussions. I think we're aligned on everything now. Let me know if we're not. I'll get coding as soon as this is approved.