Update how violation highlight is rendered (#137)

* Update how violation highlight is rendered

- replace requestAnimationFrame approach with vanilla css. This
  has the benefit of being simpler and handling multi-line text
  much better.
- bump the version number

closes MAT-1134

* bump package version

and update changelog

* update package version

* Updates in response to review comments.

* Update indicate.js

fix a typo

Author: eschiebel

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [4.2.3] - 2023-01-30
+
+- Change the violation highlight from an absolutely positioned blue box
+  to an outline created with just CSS
+
 ## [4.1.2] - 2022-11-30
 
 - Changes to cope with the RCE being put in browser-native fullscreen

package.json

@@ -1,6 +1,6 @@
 {
   "name": "tinymce-a11y-checker",
-  "version": "4.1.2",
+  "version": "4.1.3",
   "description": "An accessibility checker plugin for TinyMCE.",
   "main": "lib/plugin.js",
   "module": "es/plugin.js",

src/components/__tests__/checker.js

@@ -33,6 +33,9 @@ beforeEach(() => {
     getContainer: () => ({
       querySelector: () => fakeIframe
     }),
+    dom: {
+      doc: document,
+    },
     on: jest.fn(),
     focus: jest.fn()
   }

src/components/checker.js

@@ -68,7 +68,6 @@ export default class Checker extends React.Component {
   }
 
   onFullscreenChange = (event) => {
-    clearIndicators(event.target)
     this.selectCurrent()
   }
 
@@ -145,11 +144,11 @@ export default class Checker extends React.Component {
   }
 
   selectCurrent() {
-    clearIndicators()
+    clearIndicators(this.props.editor.dom.doc)
     const errorNode = this.errorNode()
     if (errorNode) {
       this.getFormState()
-      dom.select(this.props.editor, errorNode)
+      dom.select(errorNode)
     } else {
       this.firstError()
     }
@@ -282,7 +281,7 @@ export default class Checker extends React.Component {
 
   handleClose() {
     this.onLeaveError()
-    clearIndicators()
+    clearIndicators(this.props.editor.dom.doc)
     this.setState({ open: false })
   }
 

src/utils/__tests__/dom.js

@@ -27,13 +27,12 @@ describe("select", () => {
   })
 
   test("scrolls the node into view", () => {
-    dom.select(editor, node, indicateFn)
+    dom.select(node, indicateFn)
     expect(node.scrollIntoView).toBeCalled()
   })
-
-  test("calls the indicator function with the editor and the node", () => {
-    dom.select(editor, node, indicateFn)
-    expect(indicateFn).toHaveBeenCalledWith(editor, node)
+  test("calls the indicator function with the node", () => {
+    dom.select(node, indicateFn)
+    expect(indicateFn).toHaveBeenCalledWith(node)
   })
 
   test("select does not throw if node is underfined or null", () => {

src/utils/__tests__/indicate.js

@@ -1,118 +1,80 @@
-import indicate, { clearIndicators } from "../indicate"
-
-let fakeEditor, fakeIframe, fakeElem, mockRAF
+import indicate, {
+  clearIndicators,
+  findChildDepth,
+  buildDepthSelector,
+  INDICATOR_STYLE,
+  A11Y_CHECKER_STYLE_ELEM_ID,
+  ensureA11yCheckerStyleElement,
+} from "../indicate"
 
 beforeEach(() => {
-  Element.prototype.getBoundingClientRect = jest.fn(() => {
-    return {
-      width: 120,
-      height: 120,
-      top: 0,
-      left: 0,
-      bottom: 0,
-      right: 0,
-    }
-  })
-
-  fakeElem = document.createElement("div")
-  fakeIframe = document.createElement("iframe")
-  fakeEditor = {
-    getContainer: () => ({
-      querySelector: () => fakeIframe,
-    }),
-  }
-
-  mockRAF = jest.spyOn(window, "requestAnimationFrame")
+  document.body.innerHTML = `
+  <p id="p1">
+    first para
+  </p>
+  <p id="p2">2nd para</p>
+  <p id="p3">
+    this is the 3nd para.
+    <span id="s1">with first span</span>
+    <span id="s2">with 2nd span</span>
+    <span id="s3">with 3rd span</span>
+    more text
+    <span id="s4">target span</span>
+    <span id="s5">last span</span>
+  </p>`
 })
 
 afterEach(() => {
   document.fullscreenElement = null
-  window.requestAnimationFrame.mockRestore()
 })
 
-describe("indicate", () => {
-  it("removes any existing indicators when run", () => {
-    mockRAF
-      .mockImplementationOnce((cb) => cb(15)) // This only allows it to happen twice, preventing an infinite loop
-      .mockImplementationOnce((cb) => cb(30))
-
-    const el = document.createElement("div")
-    el.className = "a11y-checker-selection-indicator"
-    el.id = "this_should_be_gone"
-    indicate(fakeEditor, fakeElem)
-    expect(document.getElementById("this_should_be_gone")).toBeFalsy()
+describe("findChildDepth", () => {
+  it("finds the depth of a child element in its parent", () => {
+    let depth = findChildDepth(
+      document.getElementById("p3"),
+      document.getElementById("s3")
+    )
+    expect(depth).toEqual(3)
   })
 
-  it("stops adjusting when the indicator is gone", () => {
-    mockRAF
-      .mockImplementationOnce((cb) => cb(15)) 
-      .mockImplementationOnce((cb) => {
-        document.querySelector('.a11y-checker-selection-indicator').remove()
-        cb(30)
-      })
-      .mockImplementationOnce((cb) => cb(45))
-
-    indicate(fakeEditor, fakeElem)
-    expect(mockRAF).toHaveBeenCalledTimes(2)
+  it("returns 0 if any arguments are missing", () => {
+    expect(findChildDepth(null, document.getElementById("p3"))).toEqual(0)
+    expect(findChildDepth(document.getElementById("p3"), null)).toEqual(0)
   })
+})
 
-  it("puts the indicator in the fullscreeenElement if it exists", () => {
-    mockRAF
-      .mockImplementationOnce((cb) => cb(15))
-      .mockImplementationOnce((cb) => cb(30))
-
-    const d = document.createElement('div')
-    d.id = "fullscreen_element"
-    document.body.appendChild(d)
-    document.fullscreenElement = d
-    indicate(fakeEditor, fakeElem)
-    expect(d.querySelector('.a11y-checker-selection-indicator')).toBeTruthy()
-    
+describe("buildDepthSelector", () => {
+  it("returns the css selector to the given element", () => {
+    const sel = buildDepthSelector(document.getElementById("s3"))
+    expect(sel).toEqual("body>:nth-child(3)>:nth-child(3)")
   })
 })
 
-describe("clearIndicators", () => {
-  it("removes any existing indicators when called", () => {
-    const el = document.createElement("div")
-    el.className = "a11y-checker-selection-indicator"
-    el.id = "this_should_be_gone"
-    document.body.appendChild(el)
-    clearIndicators()
-    expect(document.getElementById("this_should_be_gone")).toBeFalsy()
+describe("indicate", () => {
+  it("adds the style element to the head if it doesn't exist", () => {
+    expect(document.querySelector("style")).toBe(null)
+    indicate(document.getElementById("p1"))
+    expect(
+      document.querySelector(`style#${A11Y_CHECKER_STYLE_ELEM_ID}`)
+    ).toBeTruthy()
+    indicate(document.getElementById("p1"))
+    expect(document.querySelectorAll("style").length).toEqual(1)
   })
 
-  it("removes indicators from the passed in parent", () => {
-    const d1 = document.createElement("div")
-    d1.className = "a11y-checker-selection-indicator"
-    d1.id = "this_should_be_here"
-    document.body.appendChild(d1)
-
-    const parent = document.createElement("div")
-    const el = document.createElement("div")
-    el.className = "a11y-checker-selection-indicator"
-    el.id = "this_should_be_gone"
-    parent.appendChild(el)
-    document.body.appendChild(parent)
-    clearIndicators(parent)
-    expect(document.getElementById("this_should_be_gone")).toBeFalsy()
-    expect(document.getElementById("this_should_be_here")).toBeTruthy()
+  it("injects the css into the style element", () => {
+    indicate(document.getElementById("s3"))
+    expect(
+      document.getElementById(A11Y_CHECKER_STYLE_ELEM_ID).textContent
+    ).toEqual(`body>:nth-child(3)>:nth-child(3){${INDICATOR_STYLE}}`)
   })
+})
 
-  it("removes indicators from the fullscreenElement", () => {
-    const d1 = document.createElement("div")
-    d1.className = "a11y-checker-selection-indicator"
-    d1.id = "this_should_be_here"
-    document.body.appendChild(d1)
-
-    const parent = document.createElement("div")
-    const el = document.createElement("div")
-    el.className = "a11y-checker-selection-indicator"
-    el.id = "this_should_be_gone"
-    parent.appendChild(el)
-    document.body.appendChild(parent)
-    document.fullscreenElement = parent
-    clearIndicators()
-    expect(document.getElementById("this_should_be_gone")).toBeFalsy()
-    expect(document.getElementById("this_should_be_here")).toBeTruthy()
+describe("clearIndicators", () => {
+  it("removes the indicator css when called", () => {
+    ensureA11yCheckerStyleElement(document)
+    clearIndicators(document)
+    expect(
+      document.getElementById(A11Y_CHECKER_STYLE_ELEM_ID).textContent
+    ).toEqual("")
   })
 })

src/utils/dom.js

@@ -28,12 +28,15 @@ export function walk(node, fn, done) {
   processBatch()
 }
 
-export function select(editor, elem, indicateFn = indicate) {
+export function select(elem, indicateFn = indicate) {
   if (elem == null) {
     return
   }
-  elem.scrollIntoView()
-  indicateFn(editor, elem)
+  elem.scrollIntoView(false)
+  if (elem.ownerDocument?.documentElement) {
+    elem.ownerDocument.documentElement.scrollTop += 5 // room for the indicator highlight
+  }
+  indicateFn(elem)
 }
 
 export function prepend(parent, child) {
@@ -106,7 +109,7 @@ export function splitStyleAttribute(styleString) {
 
 export function createStyleString(styleObj) {
   let styleString = Object.keys(styleObj)
-    .map(key => `${key}:${styleObj[key]}`)
+    .map((key) => `${key}:${styleObj[key]}`)
     .join(";")
   if (styleString) {
     styleString = `${styleString};`
@@ -116,5 +119,5 @@ export function createStyleString(styleObj) {
 
 export function hasTextNode(elem) {
   const nodes = Array.from(elem.childNodes)
-  return nodes.some(x => x.nodeType === Node.TEXT_NODE)
+  return nodes.some((x) => x.nodeType === Node.TEXT_NODE)
 }