fix: prevent Dot Plot overlaps and restore X-axis context menus

[Ansagan Islamgali (ansaganie)]

Fix Dot Plot layout and interaction regressions reproduced in v2.1.3.0.

• Prevent X-axis category labels from overlapping after the visual is resized
• Remove data labels that intersect dots, including dots in neighboring columns
• Open the Power BI context menu when right-clicking X-axis categories
• Add regression coverage for reduced viewports, large values, and saved radius-15 settings

[Copilot]

Pull request overview

This PR addresses layout and interaction regressions in the Dot Plot visual by improving X-axis tick behavior, removing colliding data labels, and adding regression tests to cover reduced viewports and saved formatting scenarios.

Changes:

• Rebind X-axis rendering/behavior so ticks preserve their intended datum and support Power BI context menus on right-click.
• Remove data labels that overlap any dots (including neighboring columns) to prevent visual collisions.
• Add regression tests and supporting large-value test data, plus update the changelog to document fixes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/visual.ts	Stops overriding axis tick formatting, passes tick selection into behavior, and removes labels that overlap dots; adjusts hidden-axis title behavior.
src/behavior.ts	Adds a contextmenu handler for X-axis ticks to open the Power BI context menu for the corresponding category.
test/visualTest.ts	Adds regression tests for tick overlap in reduced viewports, tick context menus, and a saved radius-15 “no overlap” scenario.
test/visualData.ts	Adds large-value categories/values used by the new regression test.
CHANGELOG.md	Documents the overlap/context menu fixes under the current version heading.

Comments suppressed due to low confidence (1)

src/visual.ts:851

• When category axis labels are hidden, this appends a new <title> element to each tick text on every update. Because D3 axis updates typically reuse existing tick nodes, the titles can accumulate over time (unbounded DOM growth). Remove any existing titles before appending, or use a join to keep exactly one title per tick.

        if (!this.formattingSettings.categoryAxis.show.value) {
            this.xAxisSelection.selectAll(DotPlot.TickTextSelector.selectorName)
                .append("title")
                .text((index: number) => {
                    return this.data.dataGroups[index]

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/visual.ts:737

• removeLabelsOverlappingDots currently does an O(labels × dots) overlap check and calls getBoundingClientRect() for every dot (and then again for every label). For larger category counts this can become a noticeable performance bottleneck and can also force repeated layout/paint work. Since each label is bound to a DotPlotDataGroup (which includes an index), you can precompute dot rects per group once and only test dots in the same/neighboring columns (the scope described in the PR), reducing the work to ~O(dots + labels).

    private removeLabelsOverlappingDots(labels: d3Selection<SVGTextElement, DotPlotDataGroup, SVGGElement, unknown>): void {
        const dotRects: DOMRect[] = this.dotPlot
            .selectAll<SVGCircleElement, DotPlotDataPoint>("circle")
            .nodes()
            .map((dot: SVGCircleElement) => dot.getBoundingClientRect());

        labels.nodes().forEach((label: SVGTextElement) => {
            const labelRect: DOMRect = label.getBoundingClientRect();
            const overlapsDot: boolean = dotRects.some((dotRect: DOMRect) => labelRect.left < dotRect.right
                && labelRect.right > dotRect.left
                && labelRect.top < dotRect.bottom
                && labelRect.bottom > dotRect.top);

            if (overlapsDot) {
                label.remove();
            }
        });
    }

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/visual.ts:732

• The current overlap check is O(#labels × #dots) (each label scans all dot rects). This quadratic scan can scale poorly for dense visuals. A more scalable approach is to bucket dots by column (or sort dot rects by left/right) and only check dots whose horizontal span intersects the label’s rect.

        const overlappingLabels: SVGTextElement[] = labels.nodes().filter((label: SVGTextElement) => {
            const labelRect: DOMRect = label.getBoundingClientRect();

            return dotRects.some((dotRect: DOMRect) => labelRect.left < dotRect.right
                && labelRect.right > dotRect.left
                && labelRect.top < dotRect.bottom
                && labelRect.bottom > dotRect.top);

src/visual.ts:724

• removeLabelsOverlappingDots() relies on getBoundingClientRect() for every dot on every update when labels are enabled. In SVG this forces layout and can become a noticeable render-time cost with many data points (especially since update() already does multiple layout-dependent operations). Consider using data-space geometry (known x/y/r from the converter/scale) or at least reducing DOM reads to the minimal subset of dots near each label.

This issue also appears on line 726 of the same file.

    private removeLabelsOverlappingDots(labels: d3Selection<SVGTextElement, DotPlotDataGroup, SVGGElement, unknown>): void {
        const dotRects: DOMRect[] = this.dotPlot
            .selectAll<SVGCircleElement, DotPlotDataPoint>(DotPlot.CircleSelector.selectorName)
            .nodes()
            .map((dot: SVGCircleElement) => dot.getBoundingClientRect());

[Alexey (Demonkratiy)]

getBoundingClientRect() is called once per circle, and every label is then tested against every
dot. With diameter = 2 * radius + 1 and maxDots = floor(dotsTotalHeight / diameter), the circle
count is roughly categories * viewportHeight / (2 * radius + 1) - at radius 1 and many categories
that is thousands of layout reads plus an O(labels x dots) scan on every update().

Dots in a column share the same x-range and form a single vertical stack, so one rect per column
group is usually enough:

const dotRects: DOMRect[] = this.dotPlot
    .selectAll<SVGGElement, DotPlotDataGroup>(DotPlot.PlotGroupSelector.selectorName)
    .nodes()
    .map((group: SVGGElement) => group.getBoundingClientRect());

That brings it down to O(labels x categories). Caveat: the group box also covers the ~1px
ExtraDiameter gaps between stacked dots, so the check becomes marginally stricter.

[Ansagan Islamgali (ansaganie)]

Done - swapped the per-circle rects for one rect per column group, exactly as you sketched.

I kept it testing every label against every group rect rather than only the label's own column. I did try the narrower version, and it does bring a couple of labels back, but those labels then sit on the neighbouring column's dots - which is the thing this function exists to prevent in the first place.

While I was in here I measured where the labels actually go, since "labels disappear even where there is room" had been pointed at this function. On the large-value case (10 categories, radius 15, no display units, precision 5):

• 10 labels get laid out
• chartutils' own hideCollidedLabels (label vs label) takes it down to 5
• this function (label vs dot) takes it from 5 to 3

So most of the culling does not happen here at all - it is the label-to-label pass inside chartutils. Worth knowing before anyone tries to make this function less eager, because that alone cannot get past 5.

I also tried capping the label text to the column pitch so labels stop reaching into their neighbours. It genuinely works on paper: all 10 labels come back and every overlap check stays clean. But the rendered result is $9... $8... $7... across the board, every value truncated to its first digit. For a value label that is worse than showing fewer of them, so I backed it out. Ellipsising is the right answer for the category axis, which already does it, and the wrong answer for the numbers.

[Alexey (Demonkratiy)]

preventDefault() runs before this guard, so when the tick datum does not map to a category the
right-click ends up doing nothing at all: the browser menu is suppressed and no Power BI menu is
shown. Falling back to the empty-selection menu would keep the interaction alive:

if (!dataPoint) {
    const emptySelection = { measures: [], dataMap: {} };
    this.selectionManager.showContextMenu(emptySelection, { x: event.clientX, y: event.clientY });
    return;
}

Same payload as the clearCatcher handler below, so behaviour stays consistent. This would need
"xAxis tick ignores context menu events for invalid category indices" to be adjusted accordingly.

[Ansagan Islamgali (ansaganie)]

Good catch - that is exactly what it did. preventDefault() had already run by the time the guard bailed out, so the tick swallowed the browser menu and then offered nothing in its place. Right-click on an unmapped tick was a dead click.

It now falls back to the same empty-selection payload the clearCatcher handler uses, so a right-click always lands somewhere sensible.

Spec updated as you flagged: it used to assert showContextMenu was never called, and now asserts it is called once with the empty selection. Renamed it too, since "ignores context menu events" no longer describes what it does.

[Alexey (Demonkratiy)]

The label-vs-dot assertions in both new tests can only ever pass: removeLabelsOverlappingDots()
deletes every label that intersects a dot, and this check is even weaker than the implementation's
own because geometryTolerance shrinks the dot rect. So this part verifies that the fix exists
rather than that the layout is correct - it would not catch a regression where the layout degrades
and the fix simply deletes more labels (labels.length > 0 is a very loose bound).

Consider asserting the concrete expected outcome as well, e.g. an exact labels.length, or that a
specific category's label is still rendered, so over-aggressive removal is caught too.

(The tick-rect assertions further down are a different matter - those do exercise the real
truncation logic and would catch a re-introduction of the tick text override.)

[Ansagan Islamgali (ansaganie)]

Fair, and the tolerance point is the sharp end of it - shrinking the dot rect made the check weaker than the implementation's own, so it could not have failed.

Both specs now assert the concrete outcome instead. The saved radius-15 one pins labels.length to 3 and checks the largest value's label is among the survivors; the uneven-stack one pins it to 2. I measured both rather than guessing them. Over-eager removal now breaks the assertion instead of quietly passing.

Kept the tick-rect assertions as they were, per your note that they exercise the real truncation logic.

[Alexey (Demonkratiy)]

The <title> is now appended unconditionally, so it is also created when categoryAxis.show is
false. In that case tickFormat returns DotPlot.DefaultTickValue ("") and the tick lines get
opacity: 0, so the parent <text> has no geometry: the tooltip can never be triggered by hover,
and an empty <text> does not surface in the accessibility tree either. The result is dead DOM that
still carries every category string.

Suggest scoping it to the visible axis:

if (this.formattingSettings.categoryAxis.show.value) {
    this.xAxisSelection.selectAll(DotPlot.TickTextSelector.selectorName)
        .append("title")
        .text((index: number) => this.data.dataGroups[index] && this.data.dataGroups[index].category.value);
}

The show === false branch of the "X-axis > show" test would need updating (it currently asserts
titles.length === 1 there).

[Ansagan Islamgali (ansaganie)]

Agreed - and the empty tick text is what settles it. With the axis hidden tickFormat returns "", so the <text> has no box to hover and an empty <text> does not surface in the accessibility tree either. Nothing could ever reach those titles, so they were just carrying every category string around for no one.

Scoped to categoryAxis.show now.

I also clear any existing titles before appending, which covers the accumulation Copilot raised earlier in the review: the axis re-render reuses the tick nodes, so without the clear a fresh <title> was being appended on every update.

Updated the show === false branch of the "X-axis > show" spec to expect zero titles.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/visual.ts:734

• removeLabelsOverlappingDots uses raw getBoundingClientRect intersections with no tolerance. Because SVG geometry is often fractional (zoom/subpixel rounding), labels that are only within a fraction of a pixel of a dot column can be treated as overlapping and removed, causing labels to disappear inconsistently across resize/zoom/browser. Consider applying a small geometry tolerance (similar to the tests) when computing overlaps.

            return dotRects.some((dotRect: DOMRect) => labelRect.left < dotRect.right
                && labelRect.right > dotRect.left
                && labelRect.top < dotRect.bottom
                && labelRect.bottom > dotRect.top);