fix(annotations): avoid rect drift when zoom != 100%#5159
fix(annotations): avoid rect drift when zoom != 100%#5159junedchhipa merged 1 commit intoapexcharts:mainfrom
Conversation
This fixes a problem with the annotations that were not showing correctly when the chart's container has a zoom level != 100%. The text and line correctly scaled with the zoom, but the rectangle position was off.
|
Explainatory codepen. Move the slider and see the annotations breaking. |
There was a problem hiding this comment.
Pull request overview
Fixes annotation label background rectangle positioning when charts are rendered inside a container with CSS zoom != 100%.
Changes:
- Adjusts annotation background rect placement calculations to account for CSS zoom.
- Adds an E2E test and sample demonstrating annotation rendering under zoom.
- Introduces new sample source XML + vanilla HTML for the zoom scenario.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/modules/annotations/Helpers.js | Scales rect positioning/sizing logic based on measured zoom factor. |
| tests/e2e/spec/line/line-with-annotations-zoom-level.spec.js | Adds E2E coverage intended to validate rect/text alignment under zoom. |
| samples/vanilla-js/line/line-with-annotations-zoom-level.html | Adds a runnable sample with zoom: 80% wrapper. |
| samples/source/line/line-with-annotations-zoom-level.xml | Adds the sample definition for documentation/demo tooling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const gridEl = w.globals.dom.baseEl.querySelector('.apexcharts-grid') | ||
| const elGridRect = gridEl.getBoundingClientRect() | ||
| const gridBBox = gridEl.getBBox() | ||
| const zoom = elGridRect.width / gridBBox.width || 1 |
There was a problem hiding this comment.
const zoom = elGridRect.width / gridBBox.width || 1 does not guard against gridBBox.width === 0. In that case the expression becomes Infinity, and subsequent divisions by zoom collapse coordinates/sizes toward 0. Use an explicit zero-check (and consider checking both width/height if you want extra safety) so zoom reliably falls back to 1 when the BBox is degenerate.
| const zoom = elGridRect.width / gridBBox.width || 1 | |
| let zoom = 1 | |
| if (gridBBox.width) { | |
| const candidateZoom = elGridRect.width / gridBBox.width | |
| if (Number.isFinite(candidateZoom) && candidateZoom > 0) { | |
| zoom = candidateZoom | |
| } | |
| } |
| const annotationRects = await page.$$eval( | ||
| '.apexcharts-yaxis-annotations rect, .apexcharts-xaxis-annotations rect, .apexcharts-point-annotations rect', | ||
| (rects) => | ||
| rects.map((r) => ({ | ||
| x: parseFloat(r.getAttribute('x')), | ||
| y: parseFloat(r.getAttribute('y')), | ||
| width: parseFloat(r.getAttribute('width')), | ||
| height: parseFloat(r.getAttribute('height')), | ||
| })) | ||
| ) | ||
|
|
||
| const annotationLabels = await page.$$eval( | ||
| '.apexcharts-yaxis-annotations text, .apexcharts-xaxis-annotations text, .apexcharts-point-annotations text', | ||
| (texts) => | ||
| texts.map((t) => { | ||
| const bbox = t.getBBox() | ||
| return { | ||
| x: bbox.x, | ||
| y: bbox.y, | ||
| width: bbox.width, | ||
| height: bbox.height, | ||
| } | ||
| }) | ||
| ) | ||
|
|
||
| // For each annotation label with a background rect, verify the rect | ||
| // encloses the text (with some padding tolerance) | ||
| const maxPadding = 20 | ||
| annotationLabels.forEach((label, i) => { | ||
| if (label.width === 0 && label.height === 0) return | ||
| if (!annotationRects[i]) return | ||
|
|
||
| const rect = annotationRects[i] | ||
|
|
||
| // The background rect should enclose the text label | ||
| expect(rect.x).toBeLessThanOrEqual(label.x + maxPadding) | ||
| expect(rect.y).toBeLessThanOrEqual(label.y + maxPadding) | ||
| expect(rect.width).toBeGreaterThan(0) | ||
| expect(rect.height).toBeGreaterThan(0) |
There was a problem hiding this comment.
This test can pass even when backgrounds are mis-positioned: (1) it assumes rects[i] corresponds to texts[i], but the two selector lists aren’t guaranteed to align 1:1 or in the same order; (2) the assertions only check the rect’s top-left isn’t too far down/right (and allow rects that are far up/left), and never verify the rect’s right/bottom edges actually cover the label (rect.x + rect.width / rect.y + rect.height). Consider pairing rect+text via DOM relationship (same annotation group/parent) and asserting full enclosure within a tolerance on all sides. Also, skipping when !annotationRects[i] silently hides failures—if a label is expected to have a background, the test should fail when it’s missing.
| const annotationRects = await page.$$eval( | |
| '.apexcharts-yaxis-annotations rect, .apexcharts-xaxis-annotations rect, .apexcharts-point-annotations rect', | |
| (rects) => | |
| rects.map((r) => ({ | |
| x: parseFloat(r.getAttribute('x')), | |
| y: parseFloat(r.getAttribute('y')), | |
| width: parseFloat(r.getAttribute('width')), | |
| height: parseFloat(r.getAttribute('height')), | |
| })) | |
| ) | |
| const annotationLabels = await page.$$eval( | |
| '.apexcharts-yaxis-annotations text, .apexcharts-xaxis-annotations text, .apexcharts-point-annotations text', | |
| (texts) => | |
| texts.map((t) => { | |
| const bbox = t.getBBox() | |
| return { | |
| x: bbox.x, | |
| y: bbox.y, | |
| width: bbox.width, | |
| height: bbox.height, | |
| } | |
| }) | |
| ) | |
| // For each annotation label with a background rect, verify the rect | |
| // encloses the text (with some padding tolerance) | |
| const maxPadding = 20 | |
| annotationLabels.forEach((label, i) => { | |
| if (label.width === 0 && label.height === 0) return | |
| if (!annotationRects[i]) return | |
| const rect = annotationRects[i] | |
| // The background rect should enclose the text label | |
| expect(rect.x).toBeLessThanOrEqual(label.x + maxPadding) | |
| expect(rect.y).toBeLessThanOrEqual(label.y + maxPadding) | |
| expect(rect.width).toBeGreaterThan(0) | |
| expect(rect.height).toBeGreaterThan(0) | |
| const annotationPairs = await page.$$eval( | |
| '.apexcharts-yaxis-annotations text, .apexcharts-xaxis-annotations text, .apexcharts-point-annotations text', | |
| (texts) => | |
| texts.map((t) => { | |
| const labelBox = t.getBBox() | |
| let rectBox = null | |
| // Pair the label with its background rect via DOM relationship | |
| const parent = t.parentNode | |
| if (parent && typeof parent.querySelector === 'function') { | |
| const rect = parent.querySelector('rect') | |
| if (rect && typeof rect.getBBox === 'function') { | |
| const rb = rect.getBBox() | |
| rectBox = { | |
| x: rb.x, | |
| y: rb.y, | |
| width: rb.width, | |
| height: rb.height, | |
| } | |
| } | |
| } | |
| return { | |
| label: { | |
| x: labelBox.x, | |
| y: labelBox.y, | |
| width: labelBox.width, | |
| height: labelBox.height, | |
| }, | |
| rect: rectBox, | |
| } | |
| }) | |
| ) | |
| // For each visible annotation label, verify the background rect | |
| // fully encloses the text (with some padding tolerance) | |
| const maxPadding = 20 | |
| annotationPairs.forEach(({ label, rect }) => { | |
| // Skip labels that have no size (not rendered / invisible) | |
| if (label.width === 0 && label.height === 0) return | |
| // Each visible label is expected to have a background rect | |
| expect(rect).toBeTruthy() | |
| expect(rect.width).toBeGreaterThan(0) | |
| expect(rect.height).toBeGreaterThan(0) | |
| // The background rect should enclose the text label on all sides | |
| expect(rect.x).toBeLessThanOrEqual(label.x + maxPadding) | |
| expect(rect.y).toBeLessThanOrEqual(label.y + maxPadding) | |
| expect(rect.x + rect.width).toBeGreaterThanOrEqual( | |
| label.x + label.width - maxPadding | |
| ) | |
| expect(rect.y + rect.height).toBeGreaterThanOrEqual( | |
| label.y + label.height - maxPadding | |
| ) |
| const annotationRects = await page.$$eval( | ||
| '.apexcharts-yaxis-annotations rect, .apexcharts-xaxis-annotations rect, .apexcharts-point-annotations rect', | ||
| (rects) => | ||
| rects.map((r) => ({ | ||
| x: parseFloat(r.getAttribute('x')), | ||
| y: parseFloat(r.getAttribute('y')), | ||
| width: parseFloat(r.getAttribute('width')), | ||
| height: parseFloat(r.getAttribute('height')), | ||
| })) | ||
| ) | ||
|
|
||
| const annotationLabels = await page.$$eval( | ||
| '.apexcharts-yaxis-annotations text, .apexcharts-xaxis-annotations text, .apexcharts-point-annotations text', | ||
| (texts) => | ||
| texts.map((t) => { | ||
| const bbox = t.getBBox() | ||
| return { | ||
| x: bbox.x, | ||
| y: bbox.y, | ||
| width: bbox.width, | ||
| height: bbox.height, | ||
| } | ||
| }) | ||
| ) | ||
|
|
||
| // For each annotation label with a background rect, verify the rect | ||
| // encloses the text (with some padding tolerance) | ||
| const maxPadding = 20 | ||
| annotationLabels.forEach((label, i) => { | ||
| if (label.width === 0 && label.height === 0) return | ||
| if (!annotationRects[i]) return | ||
|
|
||
| const rect = annotationRects[i] | ||
|
|
||
| // The background rect should enclose the text label | ||
| expect(rect.x).toBeLessThanOrEqual(label.x + maxPadding) | ||
| expect(rect.y).toBeLessThanOrEqual(label.y + maxPadding) | ||
| expect(rect.width).toBeGreaterThan(0) | ||
| expect(rect.height).toBeGreaterThan(0) |
There was a problem hiding this comment.
This test can pass even when backgrounds are mis-positioned: (1) it assumes rects[i] corresponds to texts[i], but the two selector lists aren’t guaranteed to align 1:1 or in the same order; (2) the assertions only check the rect’s top-left isn’t too far down/right (and allow rects that are far up/left), and never verify the rect’s right/bottom edges actually cover the label (rect.x + rect.width / rect.y + rect.height). Consider pairing rect+text via DOM relationship (same annotation group/parent) and asserting full enclosure within a tolerance on all sides. Also, skipping when !annotationRects[i] silently hides failures—if a label is expected to have a background, the test should fail when it’s missing.
| const annotationRects = await page.$$eval( | |
| '.apexcharts-yaxis-annotations rect, .apexcharts-xaxis-annotations rect, .apexcharts-point-annotations rect', | |
| (rects) => | |
| rects.map((r) => ({ | |
| x: parseFloat(r.getAttribute('x')), | |
| y: parseFloat(r.getAttribute('y')), | |
| width: parseFloat(r.getAttribute('width')), | |
| height: parseFloat(r.getAttribute('height')), | |
| })) | |
| ) | |
| const annotationLabels = await page.$$eval( | |
| '.apexcharts-yaxis-annotations text, .apexcharts-xaxis-annotations text, .apexcharts-point-annotations text', | |
| (texts) => | |
| texts.map((t) => { | |
| const bbox = t.getBBox() | |
| return { | |
| x: bbox.x, | |
| y: bbox.y, | |
| width: bbox.width, | |
| height: bbox.height, | |
| } | |
| }) | |
| ) | |
| // For each annotation label with a background rect, verify the rect | |
| // encloses the text (with some padding tolerance) | |
| const maxPadding = 20 | |
| annotationLabels.forEach((label, i) => { | |
| if (label.width === 0 && label.height === 0) return | |
| if (!annotationRects[i]) return | |
| const rect = annotationRects[i] | |
| // The background rect should enclose the text label | |
| expect(rect.x).toBeLessThanOrEqual(label.x + maxPadding) | |
| expect(rect.y).toBeLessThanOrEqual(label.y + maxPadding) | |
| expect(rect.width).toBeGreaterThan(0) | |
| expect(rect.height).toBeGreaterThan(0) | |
| const annotations = await page.$$eval( | |
| '.apexcharts-yaxis-annotations text, .apexcharts-xaxis-annotations text, .apexcharts-point-annotations text', | |
| (texts) => | |
| texts.map((t) => { | |
| const bbox = t.getBBox() | |
| const parent = t.parentElement | |
| const rect = parent ? parent.querySelector('rect') : null | |
| const rectBox = rect | |
| ? { | |
| x: parseFloat(rect.getAttribute('x')), | |
| y: parseFloat(rect.getAttribute('y')), | |
| width: parseFloat(rect.getAttribute('width')), | |
| height: parseFloat(rect.getAttribute('height')), | |
| } | |
| : null | |
| return { | |
| label: { | |
| x: bbox.x, | |
| y: bbox.y, | |
| width: bbox.width, | |
| height: bbox.height, | |
| }, | |
| rect: rectBox, | |
| } | |
| }) | |
| ) | |
| // For each annotation label with a background rect, verify the rect | |
| // fully encloses the text (with some padding tolerance) | |
| const maxPadding = 20 | |
| annotations.forEach((annotation) => { | |
| const { label, rect } = annotation | |
| // Skip invisible labels | |
| if (label.width === 0 && label.height === 0) return | |
| // A visible label is expected to have a background rect | |
| expect(rect).toBeTruthy() | |
| if (!rect) return | |
| // The background rect should be non-zero in size | |
| expect(rect.width).toBeGreaterThan(0) | |
| expect(rect.height).toBeGreaterThan(0) | |
| // Left/top of rect should not be too far right/down from the label | |
| expect(rect.x).toBeLessThanOrEqual(label.x + maxPadding) | |
| expect(rect.y).toBeLessThanOrEqual(label.y + maxPadding) | |
| // Right/bottom of rect should cover the label within tolerance | |
| expect(rect.x + rect.width).toBeGreaterThanOrEqual( | |
| label.x + label.width - maxPadding | |
| ) | |
| expect(rect.y + rect.height).toBeGreaterThanOrEqual( | |
| label.y + label.height - maxPadding | |
| ) |
This fixes a problem with the annotations that were not showing correctly when the chart's container has a zoom level != 100%. The text and line correctly scaled with the zoom, but the rectangle position was off.
Type of change
Please delete options that are not relevant.
Checklist: