Fix/code highlighting split#2382
Conversation
lateefazeez
requested review from
acywatson,
fantactuka,
thegreatercurve,
trueadm,
tylerjbainbridge and
zurfyx
as code owners
facebook-github-bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| return currentNode; | ||
| } | ||
|
|
||
| export function getLastCodeHighlightNodeOfLine( |
There was a problem hiding this comment.
This one should probably go into EditorShortcuts as it's not used here
|
|
||
| const LANGUAGE_DATA_ATTRIBUTE = 'data-highlight-language'; | ||
|
|
||
| export class CodeNode extends ElementNode { |
There was a problem hiding this comment.
Do you mind extracting CodeNode & all related helpers (including $isCodeNode, $createCodeNode) into own file, and same for CodeHighlightingNode. It should make highlighting plugin file easier to navigate through
| return offset; | ||
| } | ||
|
|
||
| export function getStartOfCodeInLine(anchor: LexicalNode): { |
There was a problem hiding this comment.
This one does not seem to be used anywhere else, so no need to export it. Same applies to few other exports: try to move helpers into plugins files where they are used and only export node classes + helpers that are used as API ($isCodeNode, registerCodeHighlighting, etc). The rest can remain as local helpers without exporting
Changes
|
fantactuka
left a comment
fantactuka
left a comment
There was a problem hiding this comment.
Couple minor changes needed, but looks good otherwise 👍
| editor: LexicalEditor, | ||
| threshold: number, | ||
| ): () => void; | ||
| declare function registerCodeIndent(editor: LexicalEditor): () => void; |
There was a problem hiding this comment.
Let's remove registerCodeIndent from d.ts file for now, it is part of registerCodeHighlighting for now. I think we should eventually split them and call separately, but it'll be a breaking change so it's not urgent to do now
| declare function registerCodeHighlighting(editor: LexicalEditor): () => void; | ||
| declare function registerCodeHighlighting( | ||
| editor: LexicalEditor, | ||
| threshold: number, |
There was a problem hiding this comment.
It should be optional argument, with Infinity as default value
There was a problem hiding this comment.
yes, that will be completed as part of the codehighlighter threshold branch
|
|
||
| useEffect(() => { | ||
| return registerCodeHighlighting(editor); | ||
| return registerCodeHighlighting(editor, 50); |
There was a problem hiding this comment.
Let's have it 10000 for a playground
There was a problem hiding this comment.
Okay, will do same as above
|
Hey @lateefazeez - can we get the conflicts resolved here so we can merge? |
|
@acywatson I updated this code to resolve all conflicts, but somehow not showing up here. So I created a new PR for this. Link below |
5 participants
Split Code Highlighting Plugin
Screen.Recording.2022-06-09.at.12.44.47.PM.mov
)