Skip to content
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.

Sign up for GitHub

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

[EuiMarkdownEditor] Added placeholder prop #5151

Merged
merged 7 commits into from
Sep 9, 2021
Merged

[EuiMarkdownEditor] Added placeholder prop #5151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d32b06b
Added placeholder prop
elizabetdev Sep 7, 2021
d3b0dcf
Added CL
elizabetdev Sep 7, 2021
6268191
Merge remote-tracking branch 'upstream/master' into markdown-editor-p…
elizabetdev Sep 7, 2021
d95ca7a
Omit 'placeholder'
elizabetdev Sep 7, 2021
0c7010c
Merge remote-tracking branch 'upstream/master' into markdown-editor-p…
elizabetdev Sep 8, 2021
24bbfe4
placeholders for height demos
elizabetdev Sep 8, 2021
50a185f
Merge remote-tracking branch 'upstream/master' into markdown-editor-p…
elizabetdev Sep 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- Added `placeholder` prop to `EuiMarkdownEditor` ([#5151](https://github.com/elastic/eui/pull/5151))
- Added `.eui-textNumber` utility class to apply `tnum` font-feature setting ([#5078](https://github.com/elastic/eui/pull/5078))

## [`37.6.2`](https://github.com/elastic/eui/tree/v37.6.2)
Expand Down
1 change: 1 addition & 0 deletions src-docs/src/views/markdown_editor/markdown_editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export default () => {
<>
<EuiMarkdownEditor
aria-label="EUI markdown editor demo"
placeholder="Your markdown here..."
value={value}
onChange={setValue}
height={400}
Expand Down
1 change: 1 addition & 0 deletions src-docs/src/views/markdown_editor/markdown_editor_errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export default () => {
<EuiMarkdownEditor
aria-label="EUI markdown editor demo"
aria-describedby={errorElementId.current}
placeholder="Your markdown here..."
value={value}
onChange={setValue}
height={400}
Expand Down
11 changes: 7 additions & 4 deletions src-docs/src/views/markdown_editor/markdown_editor_height.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ export default () => {
<EuiFlexItem>
<EuiPanel color="primary">
<EuiMarkdownEditor
aria-label="EUI markdown editor demo"
aria-label="EUI markdown editor with fixed height"
placeholder="Your markdown here..."
initialViewMode="viewing"
value={value1}
onChange={setValue1}
Expand All @@ -77,7 +78,7 @@ export default () => {
<EuiFlexItem style={{ height: '600px' }}>
<EuiPanel color="primary">
<EuiMarkdownEditor
aria-label="EUI markdown editor demo"
aria-label="EUI markdown editor with full height "
initialViewMode="viewing"
value={value2}
onChange={setValue2}
Expand All @@ -93,7 +94,8 @@ export default () => {
<EuiFlexItem>
<EuiPanel color="primary">
<EuiMarkdownEditor
aria-label="EUI markdown editor demo"
aria-label="EUI markdown editor with no auto expand"
placeholder="Your markdown here..."
initialViewMode="viewing"
value={value3}
onChange={setValue3}
Expand All @@ -105,7 +107,8 @@ export default () => {
<EuiFlexItem>
<EuiPanel color="primary">
<EuiMarkdownEditor
aria-label="EUI markdown editor demo"
aria-label="EUI markdown editor with fixed and max height"
placeholder="Your markdown here..."
initialViewMode="viewing"
value={value4}
onChange={setValue4}
Expand Down
1 change: 1 addition & 0 deletions src-docs/src/views/markdown_editor/markdown_editor_with_plugins.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ export default () => {
<>
<EuiMarkdownEditor
aria-label="EUI markdown editor with plugins demo"
placeholder="Your markdown here..."
value={value}
onChange={setValue}
height={400}
Expand Down
1 change: 1 addition & 0 deletions src/components/markdown_editor/__snapshots__/markdown_editor.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ exports[`EuiMarkdownEditor is rendered 1`] = `
aria-label="aria-label"
class="euiMarkdownEditorTextArea"
id="editorId"
placeholder="placeholder"
rows="6"
style="height:calc(250px);max-height:500px"
/>
Expand Down
1 change: 1 addition & 0 deletions src/components/markdown_editor/markdown_editor.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('EuiMarkdownEditor', () => {
const component = render(
<EuiMarkdownEditor
editorId="editorId"
placeholder="placeholder"
value=""
onChange={() => null}
{...requiredProps}
Expand Down
14 changes: 12 additions & 2 deletions src/components/markdown_editor/markdown_editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ import classNames from 'classnames';
import { CommonProps, OneOf } from '../common';
import MarkdownActions, { insertText } from './markdown_actions';
import { EuiMarkdownEditorToolbar } from './markdown_editor_toolbar';
import { EuiMarkdownEditorTextArea } from './markdown_editor_text_area';
import {
EuiMarkdownEditorTextArea,
EuiMarkdownEditorTextAreaProps,
} from './markdown_editor_text_area';
import { EuiMarkdownFormat, EuiMarkdownFormatProps } from './markdown_format';
import { EuiMarkdownEditorDropZone } from './markdown_editor_drop_zone';
import { htmlIdGenerator } from '../../services/';
Expand All @@ -52,7 +55,7 @@ import { EuiResizeObserver } from '../observer/resize_observer';

type CommonMarkdownEditorProps = Omit<
HTMLAttributes<HTMLDivElement>,
'onChange'
'onChange' | 'placeholder'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure the omition of placeholder is necessary here? I wouldn't think that this is an inherit attribute of a div element.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Screenshot 2021-09-08 at 15 38 32

So I'm assuming HTMLAttributes<HTMLDivElement> includes all these "Standard HTML Attributes":

Screenshot 2021-09-08 at 15 56 42

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 @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?

Copy link
Contributor

@thompsongl thompsongl Sep 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL... But I think React is wrong 😝

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think React is wrong

I agree 🙈

> &
CommonProps & {
/** aria-label OR aria-labelledby must be set */
Expand Down Expand Up @@ -119,6 +122,11 @@ type CommonMarkdownEditorProps = Omit<
/** array defining any drag&drop handlers */
dropHandlers?: EuiMarkdownDropHandler[];

/**
* Sets the placeholder of the textarea
*/
placeholder?: EuiMarkdownEditorTextAreaProps['placeholder'];

/**
* Further extend the props applied to EuiMarkdownFormat
*/
Expand Down Expand Up @@ -200,6 +208,7 @@ export const EuiMarkdownEditor = forwardRef<
initialViewMode = MODE_EDITING,
dropHandlers = [],
markdownFormatProps,
placeholder,
...rest
},
ref
Expand Down Expand Up @@ -473,6 +482,7 @@ export const EuiMarkdownEditor = forwardRef<
onChange={(e) => onChange(e.target.value)}
value={value}
onFocus={() => setHasUnacceptedItems(false)}
placeholder={placeholder}
{...{
'aria-label': ariaLabel,
'aria-labelledby': ariaLabelledBy,
Expand Down