Repository navigation
fix: Fixed bug where annotations would appear in the wrong position - #309
Merged
Merged
Conversation
…jumping when selections are toggled
|
ShrimpCryptid
commented
Sep 16, 2026
Comment on lines
-315
to
-316
| const x = filteredCellData.values[xAxis][pointIndex]; | ||
| const y = filteredCellData.values[yAxis][pointIndex]; |
Contributor
Author
There was a problem hiding this comment.
A couple bugs were happening here.
data.indexis 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 wrongxandyvalues being derived here. (The annotation would point to the wrong cell.)- Additionally, if the
pointIndexwas now outside of the range of thefilteredCellData,xandywould beundefined. 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:
- Indexing into
allCellDatainstead offilteredCellData. - Adding in a
??nullish coalescing operator to changeundefinedtonull, allowing the check on L317 to complete. - Applied group filtering so annotations that are part of hidden groups are not shown.
Contributor
There was a problem hiding this comment.
🟡 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.
ShrimpCryptid
marked this pull request as ready for review
September 16, 2026 18:10
rugeli
approved these changes
Sep 16, 2026
meganrm
approved these changes
Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
getAnnotationsunit tests.Type of change
Please delete options that are not relevant.
Steps to Verify: