From 10c37523be8daab4e7c5b8189cc3c0fa30b3ea56 Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Thu, 1 Jul 2021 17:23:09 -0600 Subject: [PATCH 1/3] adding RFC document --- .../convergence/input-native-element-props.md | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 rfcs/convergence/input-native-element-props.md diff --git a/rfcs/convergence/input-native-element-props.md b/rfcs/convergence/input-native-element-props.md new file mode 100644 index 00000000000000..22b4990849fb33 --- /dev/null +++ b/rfcs/convergence/input-native-element-props.md @@ -0,0 +1,192 @@ +# RFC: How to handle native element props for input components + + + +--- + +@sopranopillow @ecraig12345 + +## Summary + +This RFC proposes options on how to handle the native element props for input components on `vNext`. Currently the native props for all vNext components are applied to the root element, but in the case of an input component there are native input props that should be handled as well. + + + +## Problem statement + +For most components, it makes sense to apply native props (and the ref) to the root element. + +The input components are a little unique because the actual `` *cannot* be the root element of the component, since we need various wrappers for styling. For example, rendering a `` actually does this: +```tsx +
+
{/* checkmark */} + +
+``` +Or an `` is roughly like this: +```tsx +
+ {/* optional bookend before slot */} +
{/* wrapper visually styled as an input (maybe, TBD) */} + {/* optional start slot */} + + {/* optional end slot */} +
+ {/* optional bookend after slot */} +
+``` + +Another unique thing about inputs is that to the degree possible, we'd like them to work nicely with 3rd-party form libraries, which may have APIs that expect to be able to pass native props to the component and have them applied to the actual ``. (needs more research) + +### [Formik](https://github.com/formium/formik) + +### [react-hook-form](https://github.com/react-hook-form/react-hook-form) + +Note, for this one they'd need to make a wrapper for our component because the library expects to be able to *set* `.value` on the input, which we probably don't intend to support. (Someone brought this up for v8 [here](https://github.com/microsoft/fluentui/issues/18126).) + +### [React Final Form](https://github.com/final-form/react-final-form) + + + + + +## Detailed Design or Proposal + + + +We don't have one suggested/preferred solution yet, because all of them have some significant problems. + +The examples below assume you're rendering a Checkbox and this is roughly your desired HTML output (it would also include a generated ID to associate the label and input): +```html +
+ + +
+``` + +### Option 1: All native props always applied to root (current behavior) + +Given this: +```tsx +sample +``` + +You get roughly this HTML, which is not useful: +```html +
+ + +
+``` + +So to specify a value, you'd have to use slot props: +```tsx +sample +``` + +#### Pros + +- Complete consistency between all components + +#### Cons + +- Very counterintuitive API + - We could use typings to ensure that doing e.g. `` caused a TS error, but it still seems very unnatural +- Might not work as easily with 3rd-party form libraries? (needs more research) + +### Option 2: Most native props applied to root, selected props applied to "actual" element + +Given this, and an implementation which uses a list of special props (including `checked` and `name`) that are passed to the "actual" element: +```tsx +sample +``` + +You'd get the desired HTML. + +The problem is, it's unclear where some of the other props ought to be applied, such as `id`, `className`, and event handlers. + +#### Pros + +- Better API for input components (and maybe other special-case components) + +#### Cons + +- Not clear how to determine which top-level props should be passed down to the `input` + - If `checked` and `name` (and a few others) go to the `input`, the user might expect this behavior for all props and then be surprised +- Not clear what happens if a prop may be needed in multiple places + - Example: if top-level `className` or `id` is passed to the `input`, what happens if they also want to give the root a `className` or `id`? +- Inconsistent between components (reduces API clarity) + +### Option 3 + +Let the native input element be the "root" slot, but use a `wrapper` slot as the actual root DOM element. Here's an example: + +```tsx + +
+ + + +``` + +So doing this would give the desired HTML: +```tsx +sample +``` + +And if you wanted to apply props to the wrapper element, you could use slot props: +```tsx + + sample + +``` + +Which would give you roughly this: +```html +
+ + +
+``` + +#### Pros + +- For the individual component, more obvious which props go where, and a more intuitive API +- Allows any valid prop to be applied to the root and/or the input (not just one or the other) + +#### Cons + +- Inconsistent between components, potentially leading to confusion +- Might be unclear which components implement this special handling + +### Pros and Cons + + + +(to be filled out once a solution is chosen) + +## Discarded Solutions + + + +(to be filled out once a solution is chosen) + +## Open Issues + + From 236e8d848cf8652dcbc7f34cfe998058ab915f8c Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Fri, 9 Jul 2021 16:44:42 -0600 Subject: [PATCH 2/3] Adding requested updates. --- .../convergence/input-native-element-props.md | 93 ++++++++++--------- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/rfcs/convergence/input-native-element-props.md b/rfcs/convergence/input-native-element-props.md index 22b4990849fb33..f6c4482c987434 100644 --- a/rfcs/convergence/input-native-element-props.md +++ b/rfcs/convergence/input-native-element-props.md @@ -1,16 +1,5 @@ # RFC: How to handle native element props for input components - - --- @sopranopillow @ecraig12345 @@ -25,7 +14,8 @@ This RFC proposes options on how to handle the native element props for input co For most components, it makes sense to apply native props (and the ref) to the root element. -The input components are a little unique because the actual `` *cannot* be the root element of the component, since we need various wrappers for styling. For example, rendering a `` actually does this: +The input components are a little unique because the actual `` _cannot_ be the root element of the component, since we need various wrappers for styling. For example, rendering a `` actually does this: + ```tsx
{/* checkmark */} @@ -33,7 +23,9 @@ The input components are a little unique because the actual `` *cannot* b
``` + Or an `` is roughly like this: + ```tsx
{/* optional bookend before slot */} @@ -46,33 +38,24 @@ Or an `` is roughly like this:
``` -Another unique thing about inputs is that to the degree possible, we'd like them to work nicely with 3rd-party form libraries, which may have APIs that expect to be able to pass native props to the component and have them applied to the actual ``. (needs more research) - -### [Formik](https://github.com/formium/formik) - -### [react-hook-form](https://github.com/react-hook-form/react-hook-form) - -Note, for this one they'd need to make a wrapper for our component because the library expects to be able to *set* `.value` on the input, which we probably don't intend to support. (Someone brought this up for v8 [here](https://github.com/microsoft/fluentui/issues/18126).) - -### [React Final Form](https://github.com/final-form/react-final-form) +### 3rd party Form validation libraries +Another unique thing about inputs is that to the degree possible, we'd like them to work nicely with 3rd-party form libraries, which may have APIs that expect to be able to pass native props to the component and have them applied to the actual ``. (needs more research) +#### [Formik](https://github.com/formium/formik) - +#### [React Final Form](https://github.com/final-form/react-final-form) ## Detailed Design or Proposal - - We don't have one suggested/preferred solution yet, because all of them have some significant problems. The examples below assume you're rendering a Checkbox and this is roughly your desired HTML output (it would also include a generated ID to associate the label and input): + ```html
@@ -83,11 +66,15 @@ The examples below assume you're rendering a Checkbox and this is roughly your d ### Option 1: All native props always applied to root (current behavior) Given this: + ```tsx -sample + + sample + ``` You get roughly this HTML, which is not useful: + ```html
@@ -96,6 +83,7 @@ You get roughly this HTML, which is not useful: ``` So to specify a value, you'd have to use slot props: + ```tsx sample ``` @@ -107,17 +95,27 @@ So to specify a value, you'd have to use slot props: #### Cons - Very counterintuitive API - - We could use typings to ensure that doing e.g. `` caused a TS error, but it still seems very unnatural + - We could use typings to ensure that doing e.g. `` caused a TS error, but it still seems very unnatural - Might not work as easily with 3rd-party form libraries? (needs more research) ### Option 2: Most native props applied to root, selected props applied to "actual" element Given this, and an implementation which uses a list of special props (including `checked` and `name`) that are passed to the "actual" element: + ```tsx -sample + + sample + ``` -You'd get the desired HTML. +You'd get the desired HTML: + +```html +
+ + +
+``` The problem is, it's unclear where some of the other props ought to be applied, such as `id`, `className`, and event handlers. @@ -128,9 +126,9 @@ The problem is, it's unclear where some of the other props ought to be applied, #### Cons - Not clear how to determine which top-level props should be passed down to the `input` - - If `checked` and `name` (and a few others) go to the `input`, the user might expect this behavior for all props and then be surprised + - If `checked` and `name` (and a few others) go to the `input`, the user might expect this behavior for all props and then be surprised - Not clear what happens if a prop may be needed in multiple places - - Example: if top-level `className` or `id` is passed to the `input`, what happens if they also want to give the root a `className` or `id`? + - Example: if top-level `className` or `id` is passed to the `input`, what happens if they also want to give the root a `className` or `id`? - Inconsistent between components (reduces API clarity) ### Option 3 @@ -146,11 +144,18 @@ Let the native input element be the "root" slot, but use a `wrapper` slot as the ``` So doing this would give the desired HTML: -```tsx -sample + +```html +
+
+ + + +
``` And if you wanted to apply props to the wrapper element, you could use slot props: + ```tsx sample @@ -158,8 +163,11 @@ And if you wanted to apply props to the wrapper element, you could use slot prop ``` Which would give you roughly this: + ```html
+
+
@@ -175,18 +183,13 @@ Which would give you roughly this: - Inconsistent between components, potentially leading to confusion - Might be unclear which components implement this special handling -### Pros and Cons - - - -(to be filled out once a solution is chosen) - ## Discarded Solutions - - (to be filled out once a solution is chosen) ## Open Issues - +Issues related to this RFC: + +- [Checkbox Convergence](https://github.com/microsoft/fluentui/issues/18454) +- [Input Convergence](https://github.com/microsoft/fluentui/issues/18131) From 66049ae1ec110ddc75cdb2b35e05004e9e96d6fc Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Tue, 13 Jul 2021 10:36:07 -0600 Subject: [PATCH 3/3] adding requested changes --- rfcs/convergence/input-native-element-props.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rfcs/convergence/input-native-element-props.md b/rfcs/convergence/input-native-element-props.md index f6c4482c987434..a131be102601d6 100644 --- a/rfcs/convergence/input-native-element-props.md +++ b/rfcs/convergence/input-native-element-props.md @@ -147,8 +147,6 @@ So doing this would give the desired HTML: ```html
-
-
@@ -166,8 +164,6 @@ Which would give you roughly this: ```html
-
-