Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Show vector values in tooltip #479

Merged
merged 6 commits into from
Dec 3, 2024
Merged

Conversation

ShrimpCryptid
Copy link
Contributor

@ShrimpCryptid ShrimpCryptid commented Nov 20, 2024

Problem

Closes #473, "Show vector values in tooltip".

Estimated review size: small, 10 minutes

Solution

  • Adds an additional line to the tooltip when vectors are toggled on that shows either the magnitude and angle or the XY components.
  • Adds a toggle to the Vector Arrows setting area to switch between magnitude + angle or XY components.

Type of change

  • New feature (non-breaking change which adds functionality)

Steps to Verify:

  1. Open PR preview and open a dataset: https://allen-cell-animated.github.io/timelapse-colorizer/pr-preview/pr-479/
  2. Turn on vector arrows from the viewer settings tab
  3. Hover over any cell
  4. Toggle the mode from magnitude/angle to xy components.

Screenshots (optional):

Magnitude + angle:
image

Components:
image

Copy link

github-actions bot commented Nov 20, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-12-03 22:33 UTC

Copy link

github-actions bot commented Nov 20, 2024

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 75.07% 5247 / 6989
🔵 Statements 75.07% 5247 / 6989
🔵 Functions 57.87% 136 / 235
🔵 Branches 80.81% 438 / 542
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
src/colorizer/constants.ts 92.45% 100% 66.66% 92.45% 50-53
src/colorizer/types.ts 99.09% 100% 75% 99.09% 125-126
Generated in workflow #1251

@ShrimpCryptid ShrimpCryptid added the new feature New feature or request label Nov 20, 2024
@ShrimpCryptid ShrimpCryptid self-assigned this Nov 20, 2024
@ShrimpCryptid ShrimpCryptid marked this pull request as ready for review November 21, 2024 00:30
@ShrimpCryptid ShrimpCryptid requested a review from a team as a code owner November 21, 2024 00:30
@ShrimpCryptid ShrimpCryptid requested review from meganrm, ascibisz and interim17 and removed request for a team and ascibisz November 21, 2024 00:30
Copy link

@interim17 interim17 left a comment

Choose a reason for hiding this comment

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

LGTM, works when I run it.

Some merge conflicts, but otherwise good to go.

src/Viewer.tsx Outdated
<p key="vector_components">
{vectorName}: ({numberToStringDecimal(motionDelta[0], 3, allowIntegerTruncation)},{" "}
{numberToStringDecimal(motionDelta[1], 3, allowIntegerTruncation)}) px
</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't typically like to have this much data grooming mixed up with rendering. I think this would be a lot easier to follow if you had a separate function that checked for and got the appropriate data and another one that created the

and pushed it to the array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Contributor

@meganrm meganrm left a comment

Choose a reason for hiding this comment

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

I think the code in Viewer.tsx could use some work, but I'm also pretty late on this review, so up to you

@ShrimpCryptid ShrimpCryptid merged commit 76a36a9 into main Dec 3, 2024
3 checks passed
@ShrimpCryptid ShrimpCryptid deleted the feature/vector-tooltips branch December 3, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show vector components (or angle + magnitude) in hover tooltip
3 participants