Fix&finish mistake highlight rect management

Proposed solution was not complete, match.highlights is not an array
without a good reason, so checking only match.highlights[0] is never
a good idea. Should a grammar mistake wrap across more lines of text,
match.highlights gets one element per each part of the _same_ grammar
that spans one line of text. By not checking them all, popup failed
to appear when clicking on a grammar mistake following on the next line
of text.

Furthermore, the solution only addressed BesTreeService.onClick(), while
the same logic should be applied to BesPlainTextService.onClick() too.
This commit is contained in:
Simon Rozman 2024-06-10 14:40:57 +02:00
parent 55a075b927
commit f5257f1872

View File

@ -176,13 +176,12 @@ class BesService {
* Creates grammar mistake markup in DOM. * Creates grammar mistake markup in DOM.
* *
* @param {Range} range Grammar mistake range * @param {Range} range Grammar mistake range
* @returns {Object} Client rectangles and grammar mistake highlight elements * @returns {Array} Grammar mistake highlight elements
*/ */
addMistakeMarkup(range) { addMistakeMarkup(range) {
const clientRects = range.getClientRects()
const scrollPanelRect = this.scrollPanel.getBoundingClientRect() const scrollPanelRect = this.scrollPanel.getBoundingClientRect()
let highlights = [] let highlights = []
for (let rect of clientRects) { for (let rect of range.getClientRects()) {
const highlight = document.createElement('div') const highlight = document.createElement('div')
highlight.classList.add('bes-typo-mistake') highlight.classList.add('bes-typo-mistake')
highlight.style.left = `${rect.left - scrollPanelRect.left}px` highlight.style.left = `${rect.left - scrollPanelRect.left}px`
@ -192,7 +191,7 @@ class BesService {
this.scrollPanel.appendChild(highlight) this.scrollPanel.appendChild(highlight)
highlights.push(highlight) highlights.push(highlight)
} }
return { clientRects, highlights } return highlights
} }
/** /**
@ -468,11 +467,8 @@ class BesTreeService extends BesService {
} }
} }
const { clientRects, highlights } =
this.addMistakeMarkup(range)
matches.push({ matches.push({
rects: clientRects, highlights: this.addMistakeMarkup(range),
highlights: highlights,
range: range, range: range,
match: match match: match
}) })
@ -539,10 +535,8 @@ class BesTreeService extends BesService {
repositionAllMarkup() { repositionAllMarkup() {
this.results.forEach(result => { this.results.forEach(result => {
result.matches.forEach(match => { result.matches.forEach(match => {
const { clientRects, highlights } = this.addMistakeMarkup(match.range)
match.rects = clientRects
if (match.highlights) match.highlights.forEach(h => h.remove()) if (match.highlights) match.highlights.forEach(h => h.remove())
match.highlights = highlights match.highlights = this.addMistakeMarkup(match.range)
}) })
}) })
} }
@ -696,24 +690,23 @@ class BesTreeService extends BesService {
const el = this.getBlockParent(source.targetElement || source.target) const el = this.getBlockParent(source.targetElement || source.target)
if (!el) return if (!el) return
let resultMatch
for (let result of this.results) { for (let result of this.results) {
let match = result.matches.find(match => { for (let m of result.matches) {
// This way we can check if the click was inside the rectangle of the highlight even if some content was removed or added. for (let h of m.highlights) {
// We used to check if the click was inside the stored rectangle coordinates which is not reliable because the content can change. if (
// But this way we can check if the click was inside the rectangle of the highlight even if some content was changed. BesService.isPointInRect(
// TODO: what to do with rectLists inside match.rects? source.clientX,
let rect = match.highlights[0].getBoundingClientRect() source.clientY,
return BesService.isPointInRect(source.clientX, source.clientY, rect) h.getBoundingClientRect()
}) )
if (match) { ) {
resultMatch = match this.popupCorrectionPanel(el, m, source)
break return
}
}
} }
} }
BesPopup.hide()
if (resultMatch) this.popupCorrectionPanel(el, resultMatch, source)
else BesPopup.hide()
} }
} }
@ -1179,11 +1172,8 @@ class BesPlainTextService extends BesService {
nodes[nodeIdx].node, nodes[nodeIdx].node,
matchEnd - nodes[nodeIdx].start matchEnd - nodes[nodeIdx].start
) )
const { clientRects, highlights } =
this.addMistakeMarkup(matchRange)
matches.push({ matches.push({
rects: clientRects, highlights: this.addMistakeMarkup(matchRange),
highlights: highlights,
range: matchRange, range: matchRange,
match: match match: match
}) })
@ -1264,10 +1254,8 @@ class BesPlainTextService extends BesService {
repositionAllMarkup() { repositionAllMarkup() {
this.results.forEach(result => { this.results.forEach(result => {
result.matches.forEach(match => { result.matches.forEach(match => {
const { clientRects, highlights } = this.addMistakeMarkup(match.range)
match.rects = clientRects
if (match.highlights) match.highlights.forEach(h => h.remove()) if (match.highlights) match.highlights.forEach(h => h.remove())
match.highlights = highlights match.highlights = this.addMistakeMarkup(match.range)
}) })
}) })
} }
@ -1333,8 +1321,14 @@ class BesPlainTextService extends BesService {
for (let result of this.results) { for (let result of this.results) {
for (let m of result.matches) { for (let m of result.matches) {
for (let r of m.rects) { for (let h of m.highlights) {
if (BesService.isPointInRect(source.clientX, source.clientY, r)) { if (
BesService.isPointInRect(
source.clientX,
source.clientY,
h.getBoundingClientRect()
)
) {
this.popupCorrectionPanel(result.range, m, source) this.popupCorrectionPanel(result.range, m, source)
return return
} }