RFC: How to handle native element props for input components

[sopranopillow]

Pull request checklist

• Addresses an existing issue: Fixes #0000
• Include a change request file using $ yarn change

Description of changes

This PR adds an RFC that suggests options on how to move forward with handling native element props for input components. There are pros and cons to each of the options we could follow and so feedback from the team is needed to make a final decision.

Focus areas to test

(optional)

[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 66049ae:

Sandbox	Source
Fluent UI Button	Configuration
codesandbox-react-template	Configuration
codesandbox-react-northstar-template	Configuration

[size-auditor]

Asset size changes

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

Baseline commit: 035ba0498cee783b11b31dffa0cbf595b73d6ec8 (build)

[fabricteam]

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
Avatar	mount	814	812	5000
BaseButton	mount	935	923	5000
Breadcrumb	mount	2528	2523	1000
ButtonNext	mount	550	541	5000
Checkbox	mount	1577	1621	5000
CheckboxBase	mount	1342	1326	5000
ChoiceGroup	mount	4865	4805	5000
ComboBox	mount	986	971	1000
CommandBar	mount	10353	10212	1000
ContextualMenu	mount	6344	6172	1000
DefaultButton	mount	1119	1200	5000
DetailsRow	mount	3973	3803	5000
DetailsRowFast	mount	3844	3882	5000
DetailsRowNoStyles	mount	3801	3699	5000
Dialog	mount	2239	2300	1000
DocumentCardTitle	mount	142	132	1000
Dropdown	mount	3429	3366	5000
FluentProviderNext	mount	6831	7038	5000
FocusTrapZone	mount	1825	1783	5000
FocusZone	mount	1826	1780	5000
IconButton	mount	1750	1813	5000
Label	mount	335	338	5000
Layer	mount	1776	1807	5000
Link	mount	448	463	5000
MakeStyles	mount	1720	1760	50000
MenuButton	mount	1496	1519	5000
MessageBar	mount	2001	1958	5000
Nav	mount	3346	3339	1000
OverflowSet	mount	1016	1009	5000
Panel	mount	1374	2006	1000
Persona	mount	815	820	1000
Pivot	mount	1378	1396	1000
PrimaryButton	mount	1332	1323	5000
Rating	mount	7955	7859	5000
SearchBox	mount	1398	1387	5000
Shimmer	mount	2666	2609	5000
Slider	mount	1959	1974	5000
SpinButton	mount	5025	5030	5000
Spinner	mount	413	400	5000
SplitButton	mount	3218	3141	5000
Stack	mount	499	515	5000
StackWithIntrinsicChildren	mount	1569	1569	5000
StackWithTextChildren	mount	4700	4702	5000
SwatchColorPicker	mount	10270	10272	5000
Tabs	mount	1405	1420	1000
TagPicker	mount	2556	2462	5000
TeachingBubble	mount	11611	11636	5000
Text	mount	413	450	5000
TextField	mount	1411	1390	5000
ThemeProvider	mount	1156	1154	5000
ThemeProvider	virtual-rerender	585	575	5000
Toggle	mount	838	814	5000
buttonNative	mount	105	109	5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AnimationMinimalPerf.default	448	413	1.08:1
AvatarMinimalPerf.default	212	197	1.08:1
BoxMinimalPerf.default	401	376	1.07:1
ListCommonPerf.default	709	667	1.06:1
ReactionMinimalPerf.default	416	391	1.06:1
ButtonMinimalPerf.default	181	173	1.05:1
SegmentMinimalPerf.default	388	372	1.04:1
DividerMinimalPerf.default	398	387	1.03:1
GridMinimalPerf.default	363	354	1.03:1
LabelMinimalPerf.default	423	409	1.03:1
ListMinimalPerf.default	543	529	1.03:1
TextAreaMinimalPerf.default	560	542	1.03:1
VideoMinimalPerf.default	654	638	1.03:1
AlertMinimalPerf.default	299	294	1.02:1
AttachmentMinimalPerf.default	174	170	1.02:1
FlexMinimalPerf.default	309	304	1.02:1
FormMinimalPerf.default	453	443	1.02:1
LayoutMinimalPerf.default	396	390	1.02:1
MenuMinimalPerf.default	904	886	1.02:1
SkeletonMinimalPerf.default	369	362	1.02:1
TableManyItemsPerf.default	2062	2012	1.02:1
ToolbarMinimalPerf.default	974	959	1.02:1
AttachmentSlotsPerf.default	1111	1095	1.01:1
CardMinimalPerf.default	603	598	1.01:1
CarouselMinimalPerf.default	499	495	1.01:1
ChatMinimalPerf.default	687	682	1.01:1
DialogMinimalPerf.default	783	775	1.01:1
LoaderMinimalPerf.default	727	717	1.01:1
MenuButtonMinimalPerf.default	1767	1751	1.01:1
TextMinimalPerf.default	368	365	1.01:1
ButtonOverridesMissPerf.default	1760	1759	1:1
DatepickerMinimalPerf.default	5513	5539	1:1
EmbedMinimalPerf.default	4223	4210	1:1
HeaderMinimalPerf.default	401	400	1:1
HeaderSlotsPerf.default	818	814	1:1
InputMinimalPerf.default	1273	1271	1:1
ListNestedPerf.default	607	604	1:1
SliderMinimalPerf.default	1581	1586	1:1
StatusMinimalPerf.default	714	715	1:1
TableMinimalPerf.default	432	433	1:1
CustomToolbarPrototype.default	3895	3885	1:1
AccordionMinimalPerf.default	170	172	0.99:1
CheckboxMinimalPerf.default	2750	2790	0.99:1
DropdownMinimalPerf.default	3094	3110	0.99:1
ItemLayoutMinimalPerf.default	1292	1308	0.99:1
PopupMinimalPerf.default	598	604	0.99:1
ProviderMergeThemesPerf.default	1628	1644	0.99:1
RadioGroupMinimalPerf.default	472	475	0.99:1
SplitButtonMinimalPerf.default	3969	4003	0.99:1
TooltipMinimalPerf.default	1043	1050	0.99:1
TreeMinimalPerf.default	830	840	0.99:1
ButtonSlotsPerf.default	574	585	0.98:1
RosterPerf.default	1246	1269	0.98:1
ProviderMinimalPerf.default	1018	1037	0.98:1
ChatWithPopoverPerf.default	375	386	0.97:1
DropdownManyItemsPerf.default	729	748	0.97:1
ListWith60ListItems.default	655	672	0.97:1
PortalMinimalPerf.default	179	184	0.97:1
RefMinimalPerf.default	224	230	0.97:1
IconMinimalPerf.default	632	654	0.97:1
ChatDuplicateMessagesPerf.default	304	316	0.96:1
TreeWith60ListItems.default	172	181	0.95:1
ImageMinimalPerf.default	400	430	0.93:1

[ling1726]

Could you do a survey of other react design systems and what solutions they use ? AFAIK most design systems will follow something along the lines of Option 1 or Option 2

[ecraig12345]

I'd have to dig into it a bit more for Input (and Esteban would have a better idea for Checkbox) but I don't believe that's true. IIRC from the research I did, they appear to most commonly use either option 2 (combination) or possibly tending to apply the props just to the input not the root. But to confirm that I'd need to make a bunch of codesandboxes and look at their props and DOM.

[bsunderhus]

So far IMO I'd go for Option 1. seems more consistent with all other converged components.
But yeah, the arguments of counter intuitivity is valid.

Perhaps you could faltten some obvious properties in the root element and manually transfer it to the slot.

[Hotell]

you could faltten some obvious properties in the root element and manually transfer it to the slot.

unless this is consistent and explicitly stated for all vNext controls, I'd avoid any "special case" magic

[ecraig12345]

Perhaps you could faltten some obvious properties in the root element and manually transfer it to the slot.

@bsunderhus Define "obvious"? I haven't been able to come up with a definition that relies on anything besides intuition, which is subjective and varies between people.

[bsunderhus]

Yeah, you're right. This is too much subjective.

[behowell]

I checked a few other systems that I could readily get a working sandbox for their text Input component, and the consensus seemed to be something like Option 2, only slightly modified in that all native props go to the input element, with a small number of exceptions that go to the wrapper (e.g. className).

I tested by adding a few props to the checkbox component and seeing what DOM element they ended up on (using data-test as a proxy for "any arbitrary prop"). The results were similar for most, but I've marked the differences with [*]

	name	id	className	aria-label	data-test
Ant	input	input	wrapper	input	input
Carbon	input	input	input [*]	input	input
Fabric v8	input	input	wrapper	-	input
Northstar v0	input	input	wrapper	input	wrapper [*]

• Carbon put className on the input element for text Input component in that table, but for the Checkbox component, it put className on the label element (not the wrapper, nor the input elements).

[ling1726]

To a certain extent this would happen anyway right ? since the component itself would most likely be exposing some kind of value prop for controlling

[layershifter]

I am in favor of "Option 2" as it allows the simplest integration with validation libraries, it's one of the most common scenarios.

Another thing to mention is that a list of HTML attributes is finite, https://github.com/Semantic-Org/Semantic-UI-React/blob/abaf123cdd0dacaa260ecdd1920a3383baa2ba4e/src/lib/htmlPropsUtils.js#L3-L36.

---

The main question then is how to pass props .root element? 🤔 Our answer could be: "There is no way" 😧 I was asked about that id and ref few times, in this case I suggested to wrap Input with an additional div:

function App() {
  const wrapperRef = React.useRef();

  React.useEffect(() => {
    console.log(wrapperRef.current.firstChild); // returns a wrapping "div"
  }, []);

  return (
    <div id="wrapper" ref={wrapperRef}>
      <Input />
    </div>
  );
}

[theerebuss]

I like the idea of having Option 2 for easy integration with form validators like Formik. I would also assume that my Input or Checkbox component's root component be the input element, just like select would be for a Select component and button for a Button component, as components like these are either styling the native elements or adding additional functionality to them.
The lack of consistency with the rest of the library is a valid concern, but, given that this is also an edge case, I think it's justified.

[Hotell]

q: will this RFC tackle also react exotic handling of change/input events on input components?

Why are you asking this? -> this is important for preact users for example

[Hotell]

are we sure about checked attribute being added ? setAttribute throws error if you don't provide value. is this some buggy react-dom behaviour ?

[ecraig12345]

Hmm looks like it's not applied. So we should update the resulting HTML, but it doesn't change the point, which is that spreading the attribute there is not useful. https://codepen.io/ecraig12345/pen/mdmOjqL

[Hotell]

you could faltten some obvious properties in the root element and manually transfer it to the slot.

unless this is consistent and explicitly stated for all vNext controls, I'd avoid any "special case" magic

[Hotell]

from my experience hook-form is a must support - most lightweight/powerful solution on the market

[ecraig12345]

How would you integrate this with standard controlled/uncontrolled value behavior?

[Hotell]

regarding proper support of various libraries/frameworks - it might be good idea to publish fluent wrappers as separate packages for those particular solutions to make adoption super easy

[Hotell]

I'd go with option 1 or option 3.

while option 1 seems to be counter intuitive from DX perspective:

• users will/should be familiar with our slotting system as it's omnipresent everywhere in vNext, so "that shouldn't be such a big issue
• as mention, we can provide wrappers for particular 3rd party validation framework

In general it would be nice to get some solid data set (feedback) from internal customers and open source community on what approach should we adopt/is less "evil".

[ecraig12345]

q: will this RFC tackle also react exotic handling of change/input events on input components?

Why are you asking this? -> this is important for preact users for example

AFAIK there's been little/no discussion of Preact support. From what I understand, this would require specific efforts and possibly some changes in approach, and certainly tests to ensure we don't accidentally break it in the future. So we shouldn't start attempting to design/optimize for Preact unless we decide to officially support it.

[ecraig12345]

I am in favor of "Option 2" as it allows the simplest integration with validation libraries, it's one of the most common scenarios.

@layershifter But how do you decide which props to cherry-pick? How do we make this obvious to users and ensure that it's semi-consistent between relevant components?

Another thing to mention is that a list of HTML attributes is finite, https://github.com/Semantic-Org/Semantic-UI-React/blob/abaf123cdd0dacaa260ecdd1920a3383baa2ba4e/src/lib/htmlPropsUtils.js#L3-L36.

What about data- attributes?

[ecraig12345]

I like the idea of having Option 2 for easy integration with form validators like Formik. I would also assume that my Input or Checkbox component's root component be the input element, just like select would be for a Select component and button for a Button component, as components like these are either styling the native elements or adding additional functionality to them.

@andrefcdias As far as I'm aware, it's not possible to have the input be the root of Input or Checkbox. input can't have children, and we need multiple elements for various features of the components: currently Checkbox has a built-in label, and Input has various extra features and styling that require multiple elements. I suspect this will also be the case with Select.

[ling1726]

Just another wild idea: Would it make sense to only ship input wrappers in Fluent ? i.e. some API like:

import { InputField } from '@fluentui/react-components'

<InputField>
  <input /> // naive input
</InputField>

We can apply our styling and behaviours to the wrapper and simply clone the child and apply any behaviours directly on the input like we do on Tooltip or MenuTrigger

Users could be happy since they will be using the native input most of the time and they can create their on <Input /> components with whatever of the options, if it suits then

[behowell]

How about a slight modification to Option 3:

• Modify the internal slots API to allow a component to specify the name of a slot that gets the native props that are specified directly on the component's props. It would be the root slot by default, but in this case, we'd override it to be the input slot instead.
• The className prop would be special-cased to always go to the root slot.
• Make root a real slot for every component, so you could write root={{ id: 'foo' }} to set the id of the root slot.

The result would be that components could choose the most logical slot to receive the native props for that component, but there would always be an escape hatch for a user to specify props on any slot.

E.g.:

<Checkbox id="foo" /> {/* The input element gets id="foo" */}
<Checkbox input={{ id: 'foo' }} /> {/* Same as above, just being more explicit */}
<Checkbox root={{ id: 'bar' }} /> {/* The root element gets id="bar" */}

[Hotell]

Just another wild idea: Would it make sense to only ship input wrappers in Fluent ? i.e. some API like:

import { InputField } from '@fluentui/react-components'

<InputField>
  <input /> // naive input
</InputField>

We can apply our styling and behaviours to the wrapper and simply clone the child and apply any behaviours directly on the input like we do on Tooltip or MenuTrigger

Users could be happy since they will be using the native input most of the time and they can create their on <Input /> components with whatever of the options, if it suits then

I have to say I like this idea. robust design systems, like angular material, are using same approach https://material.angular.io/components/input/overview

[Hotell]

q: will this RFC tackle also react exotic handling of change/input events on input components?
Why are you asking this? -> this is important for preact users for example

AFAIK there's been little/no discussion of Preact support. From what I understand, this would require specific efforts and possibly some changes in approach, and certainly tests to ensure we don't accidentally break it in the future. So we shouldn't start attempting to design/optimize for Preact unless we decide to officially support it.

AFAIR one of our big consumers is using Preact. I asked this in this RFC, as input events are the one "tricky" area where folks might run into issues with just react event handling behaviors.

thanks!

[layershifter]

Just another wild idea: Would it make sense to only ship input wrappers in Fluent ? i.e. some API like:

import { InputField } from '@fluentui/react-components'

<InputField>
  <input /> // naive input
</InputField>

We can apply our styling and behaviours to the wrapper and simply clone the child and apply any behaviours directly on the input like we do on Tooltip or MenuTrigger

Users could be happy since they will be using the native input most of the time and they can create their on <Input /> components with whatever of the options, if it suits then

Props to control component

Users will need to pass defaultValue, value, onChange somewhere:

<>
  <InputField onChange={/* Should it be there? */}>
    <input onChange={/* Or should it be there? */} />
  </InputField>

  {/* Or even worse when handlers and `value` are on different JSX elements */}
  <InputField value={someVariables}>
    <input onChange={() => {}} />
  </InputField>
</>

An obvious answer will be that props should be on input so we could say that value & other controlling props are supported only on input:

// 😔 not really correct, see below
<InputField>
  <input onChange={() => {}} value={someVariables} />
</InputField>

But even InputField should support clearable scenario and this means that state should be handled inside InputField:

// ✅ this is correct
<InputField onChange={() => {}} value={someVariable}>
  <input />
</InputField>

For more complex components like NumberField (NumberField in React Spectrum) this will be also a requirement: there NumberField controls state and has additional handlers for buttons (increment/decrement).

In the same time nothing (even typings) will prevent users from doing this:

<InputField onChange={() => {}} value={someVariable}>
  <input value={otherVariable} />
</InputField>

What should win in this case, someVariable or otherVariable? 🏎 Are there ideas how to handle this? Runtime checks?

Usage in other components

I would expect that API should scale to other components. I am not sure that it will play with well with Select and for example:

<SelectField>
  {/* it's simple to handle a single child, what about multiple? */}
  <select name="cars" id="cars" />

  <SelectOption value="volvo">Volvo</option>
  <SelectOption value="saab">Saab</option>
</SelectField>

[ling1726]

clearable scenario

Could we do this with slots

<InputField clearButton={{ onClick: () => setState }} >
  <input value={state} onChange={() => setState} />
</InputField>

NumberField

Similarly you would be controlling the the input with the slots in a controlled scenario ?

SelectField

No good answer to this, it might depend on how we would need to style a native select

[ecraig12345]

Just another wild idea: Would it make sense to only ship input wrappers in Fluent ? i.e. some API like:

import { InputField } from '@fluentui/react-components'

<InputField>
  <input /> // naive input
</InputField>

It's an interesting idea but seems like solving one specific problem at the expense of overall API clarity/simplicity, especially since it's completely different from what any other React library does (as far as I've seen). And it doesn't necessarily solve the problem of clarity about where props go, as others pointed out. So I would strongly prefer not to go with this approach.

[ecraig12345]

clearable scenario

Could we do this with slots

<InputField clearButton={{ onClick: () => setState }} >
  <input value={state} onChange={() => setState} />
</InputField>

The clearable scenario probably isn't going to be built in to the base Input component due to bundle size concerns of adding a direct reference to Button (though we may have a recomposed version or wrapper which handles putting the button it in the slot and setting appropriate props).

[ecraig12345]

After a discussion in the Redmond components crew meeting on Thursday, I made a new RFC #18983 which is a modified version of this RFC with a slightly different approach (based on one of @behowell's comments), and broadening the focus beyond input components in some aspects. @ling1726 @layershifter @Hotell @bsunderhus @andrefcdias (and anyone else interested) please take a look and let me know what you think there.

[ecraig12345]

Closing this one in favor of #18983. That one isn't ready for review yet, but I'll be updating it this week to incorporate comments here and the root as a slot RFC. Thanks @sopranopillow for your work on getting this discussion started!