[EuiMarkdownEditor] Added placeholder prop
#5151
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5151/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5151/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5151/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5151/ |
| @@ -52,7 +55,7 @@ import { EuiResizeObserver } from '../observer/resize_observer'; | |||
|
|
|||
| type CommonMarkdownEditorProps = Omit< | |||
| HTMLAttributes<HTMLDivElement>, | |||
| 'onChange' | |||
| 'onChange' | 'placeholder' | |||
There was a problem hiding this comment.
Are you sure the omition of placeholder is necessary here? I wouldn't think that this is an inherit attribute of a div element.
There was a problem hiding this comment.
When I was not omitting the placeholder the prop and description were not displaying in the props table. Once I omitted I could finally see it:
So I'm assuming HTMLAttributes<HTMLDivElement> includes all these "Standard HTML Attributes":
There was a problem hiding this comment.
🤔 @thompsongl any thoughts here? placeholder is certainly not an HTML property that exists on a <div> element nor on the extended HTMLElement. Why would it be ignored from the props table if not omitted from HTMLAttributes?
There was a problem hiding this comment.
tl;dr: This is the correct way to handle cases like this.
The HTMLElement you linked to is not the same thing as the HTMLAttributes interface from React in the screenshot. By default, any HTML element in React can accept placeholder. You can see this if you add a placeholder to EuiMarkdownEditor on master right now; TypeScript is fine with it even though it isn't passed through to the correct element (textarea).
The reason it doesn't show up in the props table unless Omited from HTMLAttributes is because we purposefully don't add default HTMLAttributes extensions to the table. If we did each table would have ~250 rows. So using Omit and redefining the placeholder type is the best method we currently have to bypass the automatic omission.
There was a problem hiding this comment.
TIL... But I think React is wrong 😝
There was a problem hiding this comment.
But I think React is wrong
I agree 🙈
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5151/ |
Summary
Closes #5128.
This PR adds a
placeholderprop toEuiMarkdownEditor.Checklist
[ ] Checked in mobile[ ] Checked Code Sandbox works for the any docs examples[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes