-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Improve Documentation #2845 - lexical/utils #4047
Improve Documentation #2845 - lexical/utils #4047
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I'm really unsure as to why the e2e tests are failing. I wonder if using typedoc inline comments inside another one caused an issue, but I don't really know. |
e2es are just being flaky the vercel deployment is failing because of this:
|
packages/lexical-utils/src/index.ts
Outdated
@@ -226,22 +285,45 @@ export function $findMatchingParent( | |||
|
|||
type Func = () => void; | |||
|
|||
/** | |||
* Executes each function passed into the mergeRegister function argument. |
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.
Almost. What this actually does is returns a function that will execute all of the functions passed to mergeRegister when called. The purpose is to make it easier to register a bunch of lexical listeners and then tear them down with a single function call. For example, in a React useEffect.
This one could potentially benefit from an @example annotation
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.
This took me a minute to wrap my head around, but I think I understand how it works especially with useEffect. I hope my explanation is clear and helpful.
packages/lexical-utils/src/index.ts
Outdated
export function mergeRegister(...func: Array<Func>): () => void { | ||
return () => { | ||
func.forEach((f) => f()); | ||
}; | ||
} | ||
|
||
/** | ||
* |
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.
This attempts to resolve nested element nodes of the same type into a single node of that type. used mostly with marks/commenting
packages/lexical-utils/src/index.ts
Outdated
/** | ||
* @param node - node to be tested against the targetNode instance | ||
* @returns true if there is a match, false otherwise | ||
*/ | ||
const $isTargetNode = (node: LexicalNode | null | undefined): node is N => { |
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.
These next ones aren't exported, so let's remove the comments
packages/lexical-utils/src/index.ts
Outdated
const $isTargetNode = (node: LexicalNode | null | undefined): node is N => { | ||
return node instanceof targetNode; | ||
}; | ||
|
||
/** | ||
* Looks for an ancestor of a node of type targetNode. Ensures there are not any children of the node of the same targetNode instance |
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.
remove
packages/lexical-utils/src/index.ts
Outdated
@@ -270,6 +352,10 @@ export function registerNestedElementResolver<N extends ElementNode>( | |||
return null; | |||
}; | |||
|
|||
/** |
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.
remove
@@ -307,6 +393,12 @@ export function registerNestedElementResolver<N extends ElementNode>( | |||
return editor.registerNodeTransform(targetNode, elementNodeTransform); | |||
} | |||
|
|||
/** | |||
* Clones the editor and marks it as dirty to be reconciled. If there was a selection, |
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 wonder how this is different from setEditorState... We can leave as-is for now and polish it up later.
Hmm - looks like a prettier error in there somewhere. Can you run prettier locally to see if you can figure out what the issue is? |
Looked like some extra whitespace at the end of some sentences. Sorry... |
With Acy's help I've tried to follow his example and add inline api documentation, though I might have gone a bit overboard. Please let me know if there's anything I can do to improve the PR.