Skip to content

WIP: handle long text#664

Draft
hellasol wants to merge 12 commits into
devfrom
265-handle-long-text
Draft

WIP: handle long text#664
hellasol wants to merge 12 commits into
devfrom
265-handle-long-text

Conversation

@hellasol

@hellasol hellasol commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator

relates to #265

-> to keep the highlights when moving to next section, do ticket that tracks changes first.

@hellasol hellasol mentioned this pull request Nov 10, 2022
@lsmith77
lsmith77 self-requested a review January 12, 2023 10:10
alert.startOffset > node.textContent.length)
)
return;
// if (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we keep this?

}, [nodesWithAlerts, elementScroll, elementRect]);

useEffect(() => {
console.log('highlights', highlights);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove


useEffect(() => {
handleKeyupEvent();
// handleKeyupEvent(); //TODO

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the implication of this change?

);
handleTextAndIcon(textWithinMaxCharLength, event);
} else {
console.log('bingo');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove

};

const handleTextAndIcon = (text: string, event?: Event) => {
console.log('handleTextAndIcon', text);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove

const nodeValueLength: number = node.nodeValue.length;

textEndAbsPosition = textStartingAbsPosition + nodeValueLength - 1; //needed to keep huglights in place
// if (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we still need this?

{ node: string; index: number }[]
>([]);

const maxCharLength = 300;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

move to config and set to 2000

});
}
}
console.log('nodesWithAlertsTemp', nodesWithAlertsTemp);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove

handleTextAndIcon(nextText, event);

//PART 1: FOCUSIN: send text within max char length to API
console.log('Part 1 focusin text', nextText.length, maxCharLength);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

console.log

const textWithinMaxCharLength = getTextWithinMaxCharLength(
textDividedByNodes.indexOf(clickedNode)
);
console.log('PART 3: textDividedByNodes', textDividedByNodes);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove

merged.set(nodeWithAlerts.nodeIndex, nodeWithAlerts);
}
}, [alerts, ignoredTerms, elementXPathResult, ignoredCategoriesFromStorage]);
console.log('merged', Array.from(merged.values()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove

let textStartingAbsPosition: number = 0;
let textEndAbsPosition: number = -1;

console.log(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove

@lsmith77

Copy link
Copy Markdown
Contributor

I guess we can close this PR and delete the branch or does it still contain useful code?

@hellasol

Copy link
Copy Markdown
Collaborator Author

Keep this for reference for when we want to track the highlights

@lsmith77
lsmith77 force-pushed the dev branch 2 times, most recently from 63cfe21 to 0db4679 Compare February 9, 2024 15:45
@lsmith77
lsmith77 force-pushed the dev branch 2 times, most recently from 3acd3dd to d3428e6 Compare February 17, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants