Skip to content

add code threshold for highlighting#2371

Closed
lateefazeez wants to merge 14 commits into
facebook:mainfrom
MLH-Fellowship:feature/highlighter-threshold
Closed

add code threshold for highlighting#2371
lateefazeez wants to merge 14 commits into
facebook:mainfrom
MLH-Fellowship:feature/highlighter-threshold

Conversation

@lateefazeez

Copy link
Copy Markdown

Starter Task 1

  • Add option to highlight plugin to stop running highlighting after certain code length threshold
  • Threshold currently set to 5
    Screen Shot 2022-06-08 at 6 06 23 PM
    Screen Shot 2022-06-08 at 6 06 59 PM

@vercel

vercel Bot commented Jun 9, 2022

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
lexical ✅ Ready (Inspect) Visit Preview Jun 16, 2022 at 9:20PM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview Jun 16, 2022 at 9:20PM (UTC)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 9, 2022
Comment thread packages/lexical-code/src/index.ts Outdated
const diffRange = getDiffRange(node.getChildren(), highlightNodes);
const {from, to, nodesForReplacement} = diffRange;
if (from !== to || nodesForReplacement.length) {
console.log("Length: ", code.length)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Console.log?

Comment thread packages/lexical-code/src/index.ts Outdated
const diffRange = getDiffRange(node.getChildren(), highlightNodes);
const {from, to, nodesForReplacement} = diffRange;
if (from !== to || nodesForReplacement.length) {
console.log("Length: ", code.length)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See linter warnings

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ooops! will make sure to always take them out

Comment thread packages/lexical-code/src/index.ts Outdated
const {from, to, nodesForReplacement} = diffRange;
if (from !== to || nodesForReplacement.length) {
console.log("Length: ", code.length)
if ((from !== to || nodesForReplacement.length) && code.length <= threshold) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It'll need some update:

what we want is if code length > threshold, then convert content back into plain text, so if you have 50 chars, and type 51st, then whole code block content should convert from code highlight nodes to text nodes

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Okay got it, working on that now.


useEffect(() => {
return registerCodeHighlighting(editor);
return registerCodeHighlighting(editor, 50);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You can use 50 as a test value, but for a playground it should be 5000-10000

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Noted

@thegreatercurve

Copy link
Copy Markdown
Contributor

@lateefazeez Maybe just rebase this branch onto main again and then push it to remote? For some reason the E2E test suite doesn't to seem to have run completely.

@acywatson acywatson closed this Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants