-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[docs][examples] Update plain text and rich text examples #6187
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
5abccd2
to
c1d3d64
Compare
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.
LGTM overall! Awesome work
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.
IMO, we need to keep react-rich
as simple as possible... I can kinda justify having AutoLinkPlugin
and FloatingLinkEditorPlugin
, but why do we need all the other things here?
Idea behind react-rich
example is to serve as a good starting point for anyone who aims to create it's own Lexical baked editor. So people would add (not remove) features.
return ( | ||
<LexicalComposer initialConfig={editorConfig}> | ||
<div className="editor-container"> | ||
<ToolbarPlugin /> |
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.
I am really wondering if we need toolbar here... Shouldn't it have same features as vanilla-js
?
The more complex examples are - the harder is to support them
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.
hi, the idea is to bring to the rich example all plugins and features from the CodeSandbox example https://codesandbox.io/p/sandbox/vigilant-kate-5tncvy?file=%2Fsrc%2Findex.js%3A11%2C16 as mentioned in #5989.
Alternatively, we can create a separate rich text example with a wider range of features, wdyt?
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.
I think each feature in the example shall serve an educational purpose. So it should be there for a reason.
And the fact that it was there in the old codesandbox isn’t a good enough reason to me :)
So unless we know why those features are needed there (ex: it’s an implementation example for particular documentation article) - I would remove them.
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.
I think each feature in the example shall serve an educational purpose. So it should be there for a reason.
And the fact that it was there in the old codesandbox isn’t a good enough reason to me :)
So unless we know why those features are needed there (ex: it’s an implementation example for particular documentation article) - I would remove them.
hi, thanks for the explanation! I've just removed extra plugins from both examples to keep the examples as simple as possible, and updated the screenshot videos.
c1d3d64
to
a6181f2
Compare
a6181f2
to
cc935c0
Compare
cc935c0
to
f1f8188
Compare
f1f8188
to
3df01ba
Compare
react-plain-text
.Description
Closes #5989
Test plan
Rich text example:
rich-text.mov
Plain text example:
plain-text.mov