Skip to content

fix: Fixed bug where annotations would appear in the wrong position - #309

Merged
ShrimpCryptid merged 4 commits into
mainfrom
fix/annotations-shifting
Sep 16, 2026
Merged

ShrimpCryptid merged 4 commits into
mainfrom
fix/annotations-shifting

Conversation

@ShrimpCryptid

@ShrimpCryptid ShrimpCryptid commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Closes #298, "Highlighted cells that are not currently visible are shown in the center of the plot."

Estimated review size: small, 5-10 minutes

Solution

  • Fixed bug where selected points would appear in the wrong position on the plot when one or more groups were hidden.
    • Derived annotation positions using indices in the all cells data instead of the filtered cell data.
    • Handled out-of-range point indices.
    • Applied group filtering to annotations.
  • Added regression test to getAnnotations unit tests.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Steps to Verify:

  1. Open bugged main build.
  2. Try hiding some groups in the left-hand panel. Annotations will jump around or appear in the wrong location.
  3. Open PR preview and repeat the above steps. Points will not jump, and will disappear with their annotated group: PR preview
Before: After:
cfe-point-bug cfe-point-bug-fixed

@ShrimpCryptid ShrimpCryptid self-assigned this Sep 16, 2026
@ShrimpCryptid ShrimpCryptid added the bug Something isn't working label Sep 16, 2026
@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
PR Preview Action v1.6.3
Preview removed because the pull request was closed.
2026-09-16 23:47 UTC

Comment on lines -315 to -316
const x = filteredCellData.values[xAxis][pointIndex];
const y = filteredCellData.values[yAxis][pointIndex];

@ShrimpCryptid ShrimpCryptid Sep 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple bugs were happening here.

  1. data.index is an index into the full cell data array and not the filtered cell data array. This meant that, if some cells were hidden, the indices of the cells after those points would shift, leading to the wrong x and y values being derived here. (The annotation would point to the wrong cell.)
  2. Additionally, if the pointIndex was now outside of the range of the filteredCellData, x and y would be undefined. L317 only checks for null values, so the points would be set to (undefined, undefined) and show up in the dead center of the plot.

I fixed this by:

  1. Indexing into allCellData instead of filteredCellData.
  2. Adding in a ?? nullish coalescing operator to change undefined to null, allowing the check on L317 to complete.
  3. Applied group filtering so annotations that are part of hidden groups are not shown.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The core group-filtering fix lacks regression test coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes annotation positioning and visibility when plot groups are hidden.

Changes:

  • Uses unfiltered cell indices for annotation coordinates.
  • Omits annotations for excluded groups and invalid coordinates.
File summaries
File Description
src/containers/MainPlotContainer/selectors.ts Corrects annotation lookup and filtering.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/containers/MainPlotContainer/selectors.ts
@ShrimpCryptid
ShrimpCryptid marked this pull request as ready for review September 16, 2026 18:10
@ShrimpCryptid
ShrimpCryptid merged commit cdea971 into main Sep 16, 2026
2 checks passed
@ShrimpCryptid
ShrimpCryptid deleted the fix/annotations-shifting branch September 16, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Highlighted cells that are not currently visible are shown in the center of the plot

4 participants