Replace hospscotch tooltip JS library for QC Plots with tippy #1172
Conversation
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
Can we remove hopscotch as a dependency from this module? It looks like qcTrendPlotReport.jsp still references it. qcSummary.jsp does too. The latter looks like it's still actively using it. There are other references to hopscotch in the codebase - could they be replaced (they may just be CSS style names, etc)
Will do, I just replaced the qc plots data points tooltip, will replace other usage too. |
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
Code is looking good, though see comments.
We should get manual testing on this before merging. I haven't had a chance to do that yet.
| { | ||
| Locator.XPathLocator hopscotchBubble = Locator.byClass("hopscotch-bubble-container"); | ||
| return hopscotchBubble.append(Locator.byClass("hopscotch-bubble-content").append(Locator.byClass("hopscotch-content").withText())); | ||
| return Locator.tagWithClass("div", "qc-plot-hover-panel"); |
There was a problem hiding this comment.
Seems redundant with getBubble() now.
| Locator.CssLocator svgBackgrounds = Locator.css("svg g.brush rect.background"); | ||
| Locator.XPathLocator hopscotchBubble = Locator.byClass("hopscotch-bubble-container"); | ||
| Locator.XPathLocator hopscotchBubbleClose = Locator.byClass("hopscotch-bubble-close"); | ||
| Locator.XPathLocator tippyBubble = Locator.tagWithClass("div", "qc-plot-hover-panel"); |
There was a problem hiding this comment.
Should this use getBubble()? Or should that use this?
| valueSpan.className = 'qc-hover-field-value'; | ||
| valueSpan.style.flex = '1'; | ||
| valueSpan.style.wordBreak = 'break-all'; | ||
| valueSpan.innerHTML = value; |
There was a problem hiding this comment.
I don't see where HTML encoding is happening. Should this be innerText, or is the caller responsible for encoding (and actually doing it)?
2 participants
Rationale
https://github.com/LabKey/internal-issues/issues/787
Related Pull Requests
Changes