Allow passing props to value input in color stop popover#4669
Allow passing props to value input in color stop popover#4669flash1293 merged 5 commits intoelastic:masterfrom
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4669/ |
CHANGELOG.md
Outdated
| @@ -1,5 +1,7 @@ | |||
| ## [`master`](https://github.com/elastic/eui/tree/master) | |||
|
|
|||
| - Added `inputValueProps` prop to `EuiColorStops` | |||
There was a problem hiding this comment.
| - Added `inputValueProps` prop to `EuiColorStops` | |
| - Added `inputValueProps` prop to `EuiColorStops` ([#4669](https://github.com/elastic/eui/pull/4669)) |
| {...{ | ||
| ...(valueInputProps || {}), |
There was a problem hiding this comment.
Interesting syntax. Was there a reason why not to simple leave the props as is and do {...valueInputProps}?
|
@cchaos No reason, that's way better, thanks. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4669/ |
cchaos
left a comment
There was a problem hiding this comment.
I agree, I don't think this needs a specific example. Thanks for the PR!
CHANGELOG.md
Outdated
| - Expanded `display` prop of `EuiCard` to inherit `color` values from `EuiPanel` ([#4649](https://github.com/elastic/eui/pull/4649)) | ||
| - Added `element` prop to `EuiPanel` for forcing to `div` or `button` ([#4649](https://github.com/elastic/eui/pull/4649)) | ||
| - Increased padding on `EuiCheckableCard` with refactor to use `EuiSplitPanel` ([#4649](https://github.com/elastic/eui/pull/4649)) | ||
| - Added `inputValueProps` prop to `EuiColorStops` ([#4669](https://github.com/elastic/eui/pull/4669)) |
There was a problem hiding this comment.
Just noticed a mismatch in the prop name here
| - Added `inputValueProps` prop to `EuiColorStops` ([#4669](https://github.com/elastic/eui/pull/4669)) | |
| - Added `valueInputProps` prop to `EuiColorStops` ([#4669](https://github.com/elastic/eui/pull/4669)) |
There was a problem hiding this comment.
Haha, good catch!
| closePopover: () => void; | ||
| 'data-index'?: string; | ||
| 'aria-valuetext'?: string; | ||
| valueInputProps?: Partial<EuiFieldNumberProps>; |
There was a problem hiding this comment.
I keep waffling on whether we should be omitting the props that we don't let the consumer override here instead of just the simple Partial<>. I feel like it would make it clear to the TS consumer why valueInputProps.isInvalid isn't manually working.
| showAlpha?: EuiColorPickerProps['showAlpha']; | ||
| /** | ||
| * Props passed to the value input field in the color stop popover. | ||
| * Can be used to configure functionality like append or prepend. |
That's a good idea, I added that to the type props of the color stop component itself. In the docs it's expanded like this: So it's not immediately obvious which props will work, but Typescript will help when actually using it - IMHO it's worth it. |
thompsongl
left a comment
There was a problem hiding this comment.
Thanks, @flash1293! LGTM
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_4669/ |
Summary
Fixes #4587
This PR adds the ability to pass props to the value input element in the color stop thumb popover from the consumer side. This can be used to tweak the appearance a bit (e.g. using
appendto show a unit of the color scale)I didn't add a documentation example as it doesn't seem important enough, but I can do so if desired
Checklist
- [ ] Added documentation- [ ] Checked Code Sandbox works for the any docs examples- [ ] Checked for accessibility including keyboard-only and screenreader modes