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

Make placeholder accessible #29

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Make placeholder accessible #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d87aed4
Make placeholder accessible
zurfyx May 24, 2024
e94e1e9
[skip ci]
korbit-ai[bot] Aug 15, 2024
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
15 changes: 10 additions & 5 deletions examples/react-rich/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ import ExampleTheme from './ExampleTheme';
import ToolbarPlugin from './plugins/ToolbarPlugin';
import TreeViewPlugin from './plugins/TreeViewPlugin';

function Placeholder() {
return <div className="editor-placeholder">Enter some rich text...</div>;
}
const placeholder = 'Enter some rich text...';

const editorConfig = {
namespace: 'React.js Demo',
Expand All @@ -38,8 +36,15 @@ export default function App() {
<ToolbarPlugin />
<div className="editor-inner">
<RichTextPlugin
contentEditable={<ContentEditable className="editor-input" />}
placeholder={<Placeholder />}
contentEditable={
<ContentEditable
className="editor-input"
placeholder={
<div className="editor-placeholder">{placeholder}</div>
}
aria-placeholder={placeholder}
/>
Comment on lines +40 to +46
Copy link
Author

Choose a reason for hiding this comment

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

category Functionality

I noticed that we're passing aria-placeholder as a prop to the ContentEditable component. While this is a step in the right direction for accessibility, we should verify if ContentEditable supports this attribute directly. If it doesn't, we might need to modify the ContentEditable component to ensure that the aria-placeholder is correctly applied to the underlying HTML element. Could you please check the ContentEditable implementation and make sure this attribute is being correctly utilized?

Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.

}
ErrorBoundary={LexicalErrorBoundary}
/>
<HistoryPlugin />
Expand Down
10 changes: 3 additions & 7 deletions packages/lexical-playground/src/Editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ import TreeViewPlugin from './plugins/TreeViewPlugin';
import TwitterPlugin from './plugins/TwitterPlugin';
import YouTubePlugin from './plugins/YouTubePlugin';
import ContentEditable from './ui/ContentEditable';
import Placeholder from './ui/Placeholder';

const skipCollaborationInit =
// @ts-expect-error
Expand All @@ -94,12 +93,11 @@ export default function Editor(): JSX.Element {
},
} = useSettings();
const isEditable = useLexicalEditable();
const text = isCollab
const placeholder = isCollab
? 'Enter some collaborative rich text...'
: isRichText
? 'Enter some rich text...'
: 'Enter some plain text...';
const placeholder = <Placeholder>{text}</Placeholder>;
const [floatingAnchorElem, setFloatingAnchorElem] =
useState<HTMLDivElement | null>(null);
const [isSmallWidthViewport, setIsSmallWidthViewport] =
Expand Down Expand Up @@ -168,11 +166,10 @@ export default function Editor(): JSX.Element {
contentEditable={
<div className="editor-scroller">
<div className="editor" ref={onRef}>
<ContentEditable />
<ContentEditable placeholder={placeholder} />
</div>
</div>
}
placeholder={placeholder}
ErrorBoundary={LexicalErrorBoundary}
/>
<MarkdownShortcutPlugin />
Expand Down Expand Up @@ -224,8 +221,7 @@ export default function Editor(): JSX.Element {
) : (
<>
<PlainTextPlugin
contentEditable={<ContentEditable />}
placeholder={placeholder}
contentEditable={<ContentEditable placeholder={placeholder} />}
ErrorBoundary={LexicalErrorBoundary}
/>
<HistoryPlugin externalHistoryState={historyState} />
Expand Down
12 changes: 5 additions & 7 deletions packages/lexical-playground/src/nodes/ImageComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ import MentionsPlugin from '../plugins/MentionsPlugin';
import TreeViewPlugin from '../plugins/TreeViewPlugin';
import ContentEditable from '../ui/ContentEditable';
import ImageResizer from '../ui/ImageResizer';
import Placeholder from '../ui/Placeholder';
import {EmojiNode} from './EmojiNode';
import {$isImageNode} from './ImageNode';
import {KeywordNode} from './KeywordNode';
Expand Down Expand Up @@ -452,12 +451,11 @@ export default function ImageComponent({
)}
<RichTextPlugin
contentEditable={
<ContentEditable className="ImageNode__contentEditable" />
}
placeholder={
<Placeholder className="ImageNode__placeholder">
Enter a caption...
</Placeholder>
<ContentEditable
placeholder="Enter a caption..."
placeholderClassName="ImageNode__placeholder"
className="ImageNode__contentEditable"
/>
}
ErrorBoundary={LexicalErrorBoundary}
/>
Expand Down
12 changes: 5 additions & 7 deletions packages/lexical-playground/src/nodes/InlineImageNode/InlineImageComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import LinkPlugin from '../../plugins/LinkPlugin';
import Button from '../../ui/Button';
import ContentEditable from '../../ui/ContentEditable';
import {DialogActions} from '../../ui/Dialog';
import Placeholder from '../../ui/Placeholder';
import Select from '../../ui/Select';
import TextInput from '../../ui/TextInput';
import {$isInlineImageNode, InlineImageNode} from './InlineImageNode';
Expand Down Expand Up @@ -388,12 +387,11 @@ export default function InlineImageComponent({
<LinkPlugin />
<RichTextPlugin
contentEditable={
<ContentEditable className="InlineImageNode__contentEditable" />
}
placeholder={
<Placeholder className="InlineImageNode__placeholder">
Enter a caption...
</Placeholder>
<ContentEditable
placeholder="Enter a caption..."
placeholderClassName="InlineImageNode__placeholder"
className="InlineImageNode__contentEditable"
/>
}
ErrorBoundary={LexicalErrorBoundary}
/>
Expand Down
12 changes: 5 additions & 7 deletions packages/lexical-playground/src/nodes/StickyComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {createWebsocketProvider} from '../collaboration';
import {useSharedHistoryContext} from '../context/SharedHistoryContext';
import StickyEditorTheme from '../themes/StickyEditorTheme';
import ContentEditable from '../ui/ContentEditable';
import Placeholder from '../ui/Placeholder';
import {$isStickyNode} from './StickyNode';

type Positioning = {
Expand Down Expand Up @@ -254,12 +253,11 @@ export default function StickyComponent({
)}
<PlainTextPlugin
contentEditable={
<ContentEditable className="StickyNode__contentEditable" />
}
placeholder={
<Placeholder className="StickyNode__placeholder">
What's up?
</Placeholder>
<ContentEditable
placeholder="What's up?"
placeholderClassName="StickyNode__placeholder"
className="StickyNode__contentEditable"
/>
}
ErrorBoundary={LexicalErrorBoundary}
/>
Expand Down
6 changes: 3 additions & 3 deletions packages/lexical-playground/src/plugins/CommentPlugin/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ import useModal from '../../hooks/useModal';
import CommentEditorTheme from '../../themes/CommentEditorTheme';
import Button from '../../ui/Button';
import ContentEditable from '../../ui/ContentEditable';
import Placeholder from '../../ui/Placeholder';

export const INSERT_INLINE_COMMAND: LexicalCommand<void> = createCommand(
'INSERT_INLINE_COMMAND',
Expand Down Expand Up @@ -168,8 +167,9 @@ function PlainTextEditor({
<LexicalComposer initialConfig={initialConfig}>
<div className="CommentPlugin_CommentInputBox_EditorContainer">
<PlainTextPlugin
contentEditable={<ContentEditable className={className} />}
placeholder={<Placeholder>{placeholder}</Placeholder>}
contentEditable={
<ContentEditable placeholder={placeholder} className={className} />
}
ErrorBoundary={LexicalErrorBoundary}
/>
<OnChangePlugin onChange={onChange} />
Expand Down
21 changes: 21 additions & 0 deletions packages/lexical-playground/src/ui/ContentEditable.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,24 @@
padding-right: 8px;
}
}

.ContentEditable__placeholder {
font-size: 15px;
color: #999;
overflow: hidden;
position: absolute;
text-overflow: ellipsis;
top: 8px;
left: 8px;
right: 8px;
user-select: none;
white-space: nowrap;
display: inline-block;
pointer-events: none;
}
@media (max-width: 1025px) {
.Placeholder__root {
left: 8px;
right: 8px;
}
}
24 changes: 20 additions & 4 deletions packages/lexical-playground/src/ui/ContentEditable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,26 @@ import './ContentEditable.css';
import {ContentEditable} from '@lexical/react/LexicalContentEditable';
import * as React from 'react';

type Props = {
className?: string;
placeholderClassName?: string;
placeholder: string;
};

export default function LexicalContentEditable({
className,
}: {
className?: string;
}): JSX.Element {
return <ContentEditable className={className || 'ContentEditable__root'} />;
placeholder,
placeholderClassName,
}: Props): JSX.Element {
return (
<ContentEditable
className={className ?? 'ContentEditable__root'}
aria-placeholder={placeholder}
placeholder={
<div className={placeholderClassName ?? 'ContentEditable__placeholder'}>
{placeholder}
</div>
}
Comment on lines +29 to +33
Copy link
Author

Choose a reason for hiding this comment

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

category Functionality

The placeholder prop is currently required, but there's no validation to ensure it's not an empty string. Consider adding a check to prevent rendering an empty placeholder, which could improve the component's robustness and prevent potential rendering issues. You could modify the placeholder rendering logic like this:

placeholder={
  placeholder ? (
    <div className={placeholderClassName ?? 'ContentEditable__placeholder'}>
      {placeholder}
    </div>
  ) : null
}

This change ensures that the placeholder is only rendered when it has content.

Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.

/>
);
}
28 changes: 0 additions & 28 deletions packages/lexical-playground/src/ui/Placeholder.css
View file
Open in desktop

This file was deleted.

22 changes: 0 additions & 22 deletions packages/lexical-playground/src/ui/Placeholder.tsx
View file
Open in desktop

This file was deleted.

8 changes: 7 additions & 1 deletion packages/lexical-react/flow/LexicalContentEditable.js.flow
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@

import * as React from 'react';

export type Props = $ReadOnly<{
export type Props = ({} | $ReadOnly<{
'aria-placeholder': string;
placeholder:
| ((isEditable: boolean) => null | React$Node)
| null
| React$Node;
}>) & $ReadOnly<{
...Partial<HTMLDivElement>,
ariaActiveDescendant?: string,
ariaAutoComplete?: string,
Expand Down
Loading