-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Input spec #20035
Merged
Merged
Input spec #20035
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
# Input Variants | ||
|
||
Input has many variants and additional features. We don't plan to implement any of these immediately, but adding notes here for future reference. | ||
|
||
## Bookends | ||
|
||
"Bookends" (the naming comes from the design spec) are bits of text on the left or right outside of the field, surrounded by a border or background color. | ||
|
||
Initially, bookends were planned to be included in the main component (as `bookendBefore`/`bookendAfter` slots) but we decided they're not essential and should be split out to simplify the DOM structure and defer some of the related open questions until later. | ||
|
||
If/when a partner needs this feature, it will likely go in some kind of wrapper component. | ||
|
||
Visual variants for the bookends are as follows (note that specific colors are just examples based on the default theme): | ||
|
||
- `filled` (default): bookends have a gray background, black text, and no border | ||
- `brand`: bookends have a brand color (such as blue or purple) background, white text, and no border | ||
- `transparent`: bookends have a transparent background, black text, and only a thin inside border to distinguish them from the field | ||
|
||
## Multi-line (`textarea`) | ||
|
||
This will tentatively be a separate component, likely sharing some internals with the basic `Input` using hooks (scheduling TBD). | ||
|
||
| Prop/concept | v8 | v0 | Proposal | | ||
| ------------------ | ---------------------------- | --- | -------------------------------------------------------- | | ||
| resizable | `resizable?: boolean` | | via native props | | ||
| auto-adjust height | `autoAdjustHeight?: boolean` | | TBD (common request but has nasty implementation issues) | | ||
|
||
Similar to both v8 and v0, passing other `textarea` native props as top-level component props will be supported. | ||
|
||
### Possibility: `useTextInput` | ||
|
||
We might want to make a hook and some types to share props and behavior between `Input` and the potential future `TextArea` (which would depend on `react-input` but with some different props and behaviors). | ||
|
||
```ts | ||
type TextInputSlots< | ||
TInput extends HTMLInputElement | HTMLTextAreaElement, | ||
TInputAttributes = TInput extends HTMLInputElement | ||
? React.InputHTMLAttributes<TInput> | ||
: React.TextAreaHTMLAttributes<TInput> | ||
> = { | ||
root: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>, HTMLElement>; | ||
input: ObjectShorthandProps<TInputAttributes, TInput>; | ||
}; | ||
``` | ||
|
||
## Validation | ||
|
||
Validation features and error messages will not be built into the new `Input`. Possible approaches: | ||
|
||
- Hooks and/or a `Field` component to provide visual consistency and proper accessibility behavior across different types of inputs. | ||
- A `Form` component for more complex scenarios involving validation across multiple fields. | ||
|
||
### In v8 | ||
|
||
```ts | ||
// relevant props only | ||
interface ITextFieldProps { | ||
// static error message | ||
errorMessage?: string | JSX.Element; | ||
// called to determine whether input is valid and show error if not | ||
// (on all changes by default; modified by validateOnFocusIn/Out) | ||
onGetErrorMessage?: (value: string) => string | JSX.Element | PromiseLike<string | JSX.Element> | undefined; | ||
// whether to validate when focus moves into input (NOT on change) | ||
validateOnFocusIn?: boolean; | ||
// whether to validate when focus moves out of input (NOT on change) | ||
validateOnFocusOut?: boolean; | ||
// whether to validate when input is initially rendered | ||
validiateOnLoad?: boolean; | ||
// wait to validate until user stops typing by this ms | ||
deferredValidationTime?: number; | ||
// called after validation completes | ||
onNotifyValidationResult?: (errorMessage: string | JSX.Element, value: string | undefined) => void; | ||
// ... | ||
} | ||
``` | ||
|
||
### In v0 | ||
|
||
TBD | ||
ecraig12345 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Password | ||
|
||
v8 supports a password field with a custom reveal password button: | ||
|
||
```tsx | ||
<TextField type="password" canRevealPassword revealPasswordAriaLabel="Show password" /> | ||
``` | ||
|
||
v0 does not appear to have a similar feature. | ||
|
||
TBD if/how we want to handle this in converged. | ||
khmakoto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Masking | ||
|
||
"Masking" refers to a text input with some pre-specified format that gets filled in as the user types, like `(___) ___-____`. | ||
|
||
In v8 it's handled by `MaskedTextField`. | ||
|
||
Masking is tricky to implement properly and bad for accessibility, so we don't plan to implement it in converged unless there's a compelling product requirement. At that point we'll also evaluate whether there are any suitable 3rd-party libraries which could handle it. | ||
ecraig12345 marked this conversation as resolved.
Show resolved
Hide resolved
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarification: this is NOT the main spec and not intended to be 100% finished yet, it's just notes about tentative plans and some comparison with v8 since that's what I'm most familiar with. A detailed spec and full comparison for any of these variants will be filled out once we're closer to implementing them.