Skip to content

Commit c8e700b

Browse files
dermatzdermatz
authored
Fix/toolbar audit highlighting (#169)
* feat: add highlight helpers and improve audit highlighting logic * fix: update toolbar active state styling for better visibility * fix: enhance audit highlighting for image elements and parent containers * fix: replace image parent background with fixed overlay for visibility * fix: improve overlay cleanup for disconnected image elements * fix: improve overlay management for image elements in audits * fix: optimize overlay management and event handling for audits
1 parent 9abe851 commit c8e700b

7 files changed

Lines changed: 186 additions & 50 deletions

File tree

‎src/view/frontend/web/css/toolbar.css‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,23 @@
403403
.mageforge-audit-low-contrast {
404404
outline: 3px solid var(--mageforge-color-red) !important;
405405
outline-offset: 2px;
406+
box-shadow: inset 0 0 0 3px var(--mageforge-color-red) !important;
407+
}
408+
409+
/* Fixed-position overlay injected over highlighted <img> elements */
410+
.mageforge-audit-img-overlay {
411+
position: fixed;
412+
pointer-events: none;
413+
background-color: rgba(239, 68, 68, 0.5);
414+
outline: 3px solid var(--mageforge-color-red);
415+
outline-offset: 0;
416+
z-index: 9999997;
417+
}
418+
419+
/* Non-image elements (inputs, text) get a colour filter for visibility */
420+
.mageforge-audit-inputs-without-label,
421+
.mageforge-audit-low-contrast {
422+
filter: brightness(0.5) sepia(1) saturate(12) hue-rotate(330deg) !important;
406423
}
407424

408425
/* ============================================================================

‎src/view/frontend/web/js/toolbar/audits/highlight.js‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/**
2+
* MageForge Toolbar – Shared highlight helpers
3+
*
4+
* Audits that mark elements by adding a CSS class use these two helpers
5+
* instead of duplicating the same logic. The CSS class is derived from the
6+
* audit key: `mageforge-audit-<key>`.
7+
*/
8+
9+
const OVERLAY_CLASS = 'mageforge-audit-img-overlay';
10+
11+
/**
12+
* Module-level registry: tracks one overlay cleanup function per <img>
13+
* element and the set of audit keys currently relying on that overlay.
14+
* Using a WeakMap means entries are automatically eligible for GC once
15+
* the image node itself is collected.
16+
*
17+
* @type {WeakMap<HTMLImageElement, { cleanup: function, keys: Set<string> }>}
18+
*/
19+
const imgOverlayRegistry = new WeakMap();
20+
21+
/**
22+
* Shared update machinery – a single ResizeObserver and capturing scroll
23+
* listener serve all active overlays, throttled via requestAnimationFrame.
24+
* Attached on the first overlay, torn down when the last one is removed.
25+
*
26+
* @type {Map<HTMLSpanElement, function>}
27+
*/
28+
const activeOverlays = new Map();
29+
let rafPending = false;
30+
let sharedRo = null;
31+
32+
function scheduleUpdate() {
33+
if (rafPending) return;
34+
rafPending = true;
35+
requestAnimationFrame(() => {
36+
rafPending = false;
37+
// Snapshot before iterating: update() calls may delete entries
38+
// (image disconnected → cleanup) while we are looping.
39+
for (const updateFn of [...activeOverlays.values()]) {
40+
updateFn();
41+
}
42+
});
43+
}
44+
45+
/**
46+
* Creates a fixed-position overlay <span> that tracks an <img> element's
47+
* position in the viewport. Shares a single RAF-throttled scroll/resize
48+
* handler across all active overlays instead of creating one per image.
49+
* Returns a cleanup function that removes the overlay and deregisters it.
50+
*
51+
* @param {HTMLImageElement} img
52+
* @returns {function} cleanup
53+
*/
54+
function createImgOverlay(img) {
55+
const overlay = document.createElement('span');
56+
overlay.className = OVERLAY_CLASS;
57+
document.body.appendChild(overlay);
58+
59+
function update() {
60+
if (!img.isConnected) {
61+
cleanup();
62+
imgOverlayRegistry.delete(img);
63+
return;
64+
}
65+
const rect = img.getBoundingClientRect();
66+
overlay.style.top = `${rect.top}px`;
67+
overlay.style.left = `${rect.left}px`;
68+
overlay.style.width = `${rect.width}px`;
69+
overlay.style.height = `${rect.height}px`;
70+
}
71+
72+
update();
73+
74+
activeOverlays.set(overlay, update);
75+
76+
// Attach shared listeners only when the first overlay is created.
77+
if (activeOverlays.size === 1) {
78+
sharedRo = new ResizeObserver(scheduleUpdate);
79+
sharedRo.observe(document.documentElement);
80+
window.addEventListener('scroll', scheduleUpdate, { passive: true, capture: true });
81+
}
82+
83+
// Named so update() can reference it before its var declaration (hoisting).
84+
function cleanup() {
85+
activeOverlays.delete(overlay);
86+
// Tear down shared listeners once no overlays remain.
87+
if (activeOverlays.size === 0) {
88+
sharedRo?.disconnect();
89+
sharedRo = null;
90+
window.removeEventListener('scroll', scheduleUpdate, { capture: true });
91+
}
92+
overlay.remove();
93+
}
94+
95+
return cleanup;
96+
}
97+
98+
/**
99+
* Removes the highlight class from all previously marked elements and
100+
* destroys any associated image overlays.
101+
*
102+
* @param {string} key - Audit key (e.g. 'images-without-alt')
103+
*/
104+
export function clearHighlight(key) {
105+
const cls = `mageforge-audit-${key}`;
106+
document.querySelectorAll(`.${cls}`).forEach(el => {
107+
el.classList.remove(cls);
108+
if (el.tagName === 'IMG') {
109+
const entry = imgOverlayRegistry.get(el);
110+
if (entry) {
111+
entry.keys.delete(key);
112+
if (entry.keys.size === 0) {
113+
entry.cleanup();
114+
imgOverlayRegistry.delete(el);
115+
}
116+
}
117+
}
118+
});
119+
}
120+
121+
/**
122+
* Highlights a set of elements by adding the audit CSS class, scrolls to the
123+
* first result, and updates the counter badge on the toolbar menu item.
124+
*
125+
* For <img> elements a fixed-position overlay is injected so the red
126+
* background is visible regardless of parent overflow or border-radius.
127+
*
128+
* @param {Element[]} elements - Elements to mark
129+
* @param {string} key - Audit key (e.g. 'images-without-alt')
130+
* @param {object} context - Alpine toolbar component instance
131+
*/
132+
export function applyHighlight(elements, key, context) {
133+
if (elements.length === 0) {
134+
context.setAuditCounterBadge(key, '0', 'success');
135+
return;
136+
}
137+
const cls = `mageforge-audit-${key}`;
138+
elements.forEach(el => {
139+
el.classList.add(cls);
140+
if (el.tagName === 'IMG') {
141+
const existing = imgOverlayRegistry.get(el);
142+
if (existing) {
143+
existing.keys.add(key);
144+
} else {
145+
imgOverlayRegistry.set(el, {
146+
cleanup: createImgOverlay(el),
147+
keys: new Set([key]),
148+
});
149+
}
150+
}
151+
});
152+
elements[0].scrollIntoView({ behavior: 'smooth', block: 'center' });
153+
context.setAuditCounterBadge(key, `${elements.length}`, 'error');
154+
}

‎src/view/frontend/web/js/toolbar/audits/images-without-alt.js‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* MageForge Toolbar Audit – Images without ALT
33
*/
44

5-
const HIGHLIGHT_CLASS = 'mageforge-audit-images-without-alt';
5+
import { applyHighlight, clearHighlight } from './highlight.js';
66

77
/** @type {import('./index.js').AuditDefinition} */
88
export default {
@@ -17,7 +17,7 @@ export default {
1717
*/
1818
run(context, active) {
1919
if (!active) {
20-
document.querySelectorAll(`.${HIGHLIGHT_CLASS}`).forEach(el => el.classList.remove(HIGHLIGHT_CLASS));
20+
clearHighlight(this.key);
2121
return;
2222
}
2323

@@ -28,13 +28,6 @@ export default {
2828
return !img.hasAttribute('alt') || img.getAttribute('alt').trim() === '';
2929
});
3030

31-
if (images.length === 0) {
32-
context.setAuditCounterBadge('images-without-alt', '0', 'success');
33-
return;
34-
}
35-
36-
images.forEach(img => img.classList.add(HIGHLIGHT_CLASS));
37-
images[0].scrollIntoView({ behavior: 'smooth', block: 'center' });
38-
context.setAuditCounterBadge('images-without-alt', `${images.length}`, 'error');
31+
applyHighlight(images, this.key, context);
3932
},
4033
};

‎src/view/frontend/web/js/toolbar/audits/images-without-dimensions.js‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* Images missing explicit width and height attributes cause Cumulative Layout Shift (CLS).
55
*/
66

7-
const HIGHLIGHT_CLASS = 'mageforge-audit-images-without-dimensions';
7+
import { applyHighlight, clearHighlight } from './highlight.js';
88

99
/** @type {import('./index.js').AuditDefinition} */
1010
export default {
@@ -19,7 +19,7 @@ export default {
1919
*/
2020
run(context, active) {
2121
if (!active) {
22-
document.querySelectorAll(`.${HIGHLIGHT_CLASS}`).forEach(el => el.classList.remove(HIGHLIGHT_CLASS));
22+
clearHighlight(this.key);
2323
return;
2424
}
2525

@@ -30,13 +30,6 @@ export default {
3030
return !img.hasAttribute('width') || !img.hasAttribute('height');
3131
});
3232

33-
if (images.length === 0) {
34-
context.setAuditCounterBadge('images-without-dimensions', '0', 'success');
35-
return;
36-
}
37-
38-
images.forEach(img => img.classList.add(HIGHLIGHT_CLASS));
39-
images[0].scrollIntoView({ behavior: 'smooth', block: 'center' });
40-
context.setAuditCounterBadge('images-without-dimensions', `${images.length}`, 'error');
33+
applyHighlight(images, this.key, context);
4134
},
4235
};

‎src/view/frontend/web/js/toolbar/audits/images-without-lazy-load.js‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* wasting bandwidth and slowing initial page load.
66
*/
77

8-
const HIGHLIGHT_CLASS = 'mageforge-audit-images-without-lazy-load';
8+
import { applyHighlight, clearHighlight } from './highlight.js';
99

1010
/** @type {import('./index.js').AuditDefinition} */
1111
export default {
@@ -20,7 +20,7 @@ export default {
2020
*/
2121
run(context, active) {
2222
if (!active) {
23-
document.querySelectorAll(`.${HIGHLIGHT_CLASS}`).forEach(el => el.classList.remove(HIGHLIGHT_CLASS));
23+
clearHighlight(this.key);
2424
return;
2525
}
2626

@@ -35,13 +35,6 @@ export default {
3535
return rect.top > viewportBottom;
3636
});
3737

38-
if (images.length === 0) {
39-
context.setAuditCounterBadge('images-without-lazy-load', '0', 'success');
40-
return;
41-
}
42-
43-
images.forEach(img => img.classList.add(HIGHLIGHT_CLASS));
44-
images[0].scrollIntoView({ behavior: 'smooth', block: 'center' });
45-
context.setAuditCounterBadge('images-without-lazy-load', `${images.length}`, 'error');
38+
applyHighlight(images, this.key, context);
4639
},
4740
};

‎src/view/frontend/web/js/toolbar/audits/inputs-without-label.js‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* to screen reader users.
66
*/
77

8-
const HIGHLIGHT_CLASS = 'mageforge-audit-inputs-without-label';
8+
import { applyHighlight, clearHighlight } from './highlight.js';
99

1010
/** @type {import('./index.js').AuditDefinition} */
1111
export default {
@@ -20,7 +20,7 @@ export default {
2020
*/
2121
run(context, active) {
2222
if (!active) {
23-
document.querySelectorAll(`.${HIGHLIGHT_CLASS}`).forEach(el => el.classList.remove(HIGHLIGHT_CLASS));
23+
clearHighlight(this.key);
2424
return;
2525
}
2626

@@ -39,13 +39,6 @@ export default {
3939
return true;
4040
});
4141

42-
if (inputs.length === 0) {
43-
context.setAuditCounterBadge('inputs-without-label', '0', 'success');
44-
return;
45-
}
46-
47-
inputs.forEach(el => el.classList.add(HIGHLIGHT_CLASS));
48-
inputs[0].scrollIntoView({ behavior: 'smooth', block: 'center' });
49-
context.setAuditCounterBadge('inputs-without-label', `${inputs.length}`, 'error');
42+
applyHighlight(inputs, this.key, context);
5043
},
5144
};

‎src/view/frontend/web/js/toolbar/audits/low-contrast-text.js‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* - 3:1 for large text (>=18pt or >=14pt bold)
77
*/
88

9-
const HIGHLIGHT_CLASS = 'mageforge-audit-low-contrast';
9+
import { applyHighlight, clearHighlight } from './highlight.js';
1010

1111
const _colorCanvas = document.createElement('canvas');
1212
_colorCanvas.width = _colorCanvas.height = 1;
@@ -164,7 +164,7 @@ export default {
164164
*/
165165
run(context, active) {
166166
if (!active) {
167-
document.querySelectorAll(`.${HIGHLIGHT_CLASS}`).forEach(el => el.classList.remove(HIGHLIGHT_CLASS));
167+
clearHighlight(this.key);
168168
return;
169169
}
170170

@@ -189,13 +189,6 @@ export default {
189189
return ratio < threshold;
190190
});
191191

192-
if (failing.length === 0) {
193-
context.setAuditCounterBadge('low-contrast-text', '0', 'success');
194-
return;
195-
}
196-
197-
failing.forEach(el => el.classList.add(HIGHLIGHT_CLASS));
198-
failing[0].scrollIntoView({ behavior: 'smooth', block: 'center' });
199-
context.setAuditCounterBadge('low-contrast-text', `${failing.length}`, 'error');
192+
applyHighlight(failing, this.key, context);
200193
},
201194
};

0 commit comments

Comments
 (0)