From 159cd516846b196fd13c640d28d55b0821bbbdb3 Mon Sep 17 00:00:00 2001 From: Aljaz Grilc Date: Fri, 24 May 2024 16:48:02 +0200 Subject: [PATCH] Improve the logic for repositioning grammar mistake highlights in response to DOM changes Refactor the click detection logic to check if the click was inside the rectangle of the highlight element, rather than relying on stored rectangle coordinates. Additionally, this update eliminates the need to attach event listeners to the entire window or the parent elements of the host element, simplifying the event handling and potentially improving performance. --- service2.js | 51 +++++++++++---------------------- styles.css | 82 ++++++++++++++++++++++++----------------------------- 2 files changed, 54 insertions(+), 79 deletions(-) diff --git a/service2.js b/service2.js index 67e97c5..d5dd8fb 100644 --- a/service2.js +++ b/service2.js @@ -8,10 +8,6 @@ */ let besServices = [] -window.addEventListener('scroll', () => - besServices.forEach(service => service.onScroll()) -) - // TODO: Window resize may cause host element(s) to move. That needs correction panel and status icon // repositioning. Also, should any parent element of our service host element move, we should reposition // correction panel and status icon. How to do this? Alas there is no PlacementObserver to monitor host @@ -153,15 +149,7 @@ class BesService { this.scrollPanel.style.top = `${-this.hostElement.scrollTop}px` // Why do we need to set left to -scrollLeft? It destroys the position of highlights - // this.scrollPanel.style.left = `${-this.hostElement.scrollLeft}px` - - // Markup is in a "position:absolute"
element requiring repositioning when scrolling host element or window. - // It is defered to reduce stress in a flood of scroll events. - if (this.scrollTimeout) clearTimeout(this.scrollTimeout) - this.scrollTimeout = setTimeout(() => { - this.updateRects() - delete this.scrollTimeout - }, 500) + this.scrollPanel.style.left = `${-this.hostElement.scrollLeft}px` } /** @@ -207,15 +195,6 @@ class BesService { return { clientRects, highlights } } - updateRects() { - this.results.forEach(result => { - result.matches.forEach(match => { - const clientRects = match.range.getClientRects() - match.rects = clientRects - }) - }) - } - /** * Tests if given coordinate is inside of a rectangle. * @@ -711,20 +690,24 @@ class BesTreeService extends BesService { const el = this.getBlockParent(source.targetElement || source.target) if (!el) return - const result = this.results.find(child => - BesTreeService.isSameParagraph(child.element, el) - ) - if (result) { - for (let m of result.matches) { - for (let r of m.rects) { - if (BesService.isPointInRect(source.clientX, source.clientY, r)) { - this.popupCorrectionPanel(el, m, source) - return - } - } + let resultMatch + for (let result of this.results) { + let match = result.matches.find(match => { + // This way we can check if the click was inside the rectangle of the highlight even if some content was removed or added. + // We used to check if the click was inside the stored rectangle coordinates which is not reliable because the content can change. + // But this way we can check if the click was inside the rectangle of the highlight even if some content was changed. + // TODO: what to do with rectLists inside match.rects? + let rect = match.highlights[0].getBoundingClientRect() + return BesService.isPointInRect(source.clientX, source.clientY, rect) + }) + if (match) { + resultMatch = match + break } } - BesPopup.hide() + + if (resultMatch) this.popupCorrectionPanel(el, resultMatch, source) + else BesPopup.hide() } } diff --git a/styles.css b/styles.css index 0f738dd..247268a 100644 --- a/styles.css +++ b/styles.css @@ -4,64 +4,32 @@ .bes-typo-mistake { border-bottom: 2px solid red; position: absolute; - z-index: 2; + z-index: 3; cursor: text; - pointer-events: none; + /* pointer-events: none; */ } .bes-correction-panel-parent { - z-index: 3; - top: 0; - left: 0; - width: 0; - height: 0; - display: inline; - float: left; position: relative; overflow: visible; - visibility: visible; + float: left; + display: inline; + width: 0px; + height: 0px; + border: none; + z-index: 1; } .bes-correction-panel { - background: transparent; - border: none; - color: transparent; - cursor: initial; - display: block; - float: initial; - margin: 0; - max-height: initial; - min-height: initial; - max-width: initial; - min-width: initial; - outline: initial; - overflow: hidden; - padding: 0; - pointer-events: none; position: relative; + overflow: hidden; + border-color: transparent; + color: transparent; + pointer-events: none; } .bes-correction-panel-scroll { - pointer-events: none; - box-shadow: initial; - box-sizing: initial; - cursor: initial; - display: block; - float: initial; - max-height: initial; - min-height: initial; - max-width: initial; - min-width: initial; - position: absolute; - height: initial; - width: initial; - animation: auto ease 0s 1 normal none running none; - background: transparent; - border-width: initial; - border-style: none; - margin: 0px; - outline: initial; - padding: 0px; + position: relative; } .bes-status-div { @@ -95,3 +63,27 @@ .bes-status-icon.bes-status-mistakes { background-image: url('images/mistake-svgrepo-com.svg'); } + +/* TODO: Styles below should be removed after testing period is over */ +.resizable { + overflow-x: scroll; + position: relative; + display: inline-block; + border: 1px solid #ccc; + width: 300px; + height: 400px; + border-radius: 3px; + background-color: #f9f9f9; +} + +.mock-content { + width: 1000px; + height: 600px; + background-color: #f9f9f9; +} + +.flex { + display: flex; + justify-content: center; + align-items: center; +}