Skip to content

Conversation

@naglepuff
Copy link
Collaborator

@naglepuff naglepuff commented Sep 16, 2025

Changes

Adds a new GeoJS layer to the spectrogram viewer that draws a draggable horizontal guide line for more easily eyeballing the minimum frequency of pulses on the spectrogram.

The frequency ruler is turned on/off by a new button above the spectrogram. The button uses the measuring tape icon. When the ruler is enabled, the button will be blue.
toolbar buttons with measuring tape icon

When the button is clicked, the ruler will be rendered. Dragging the point along the X-axis will cause the ruler to follow. The text for the frequency will update as the ruler is dragged.
image

@naglepuff naglepuff changed the title Create layer for measure tool Create a slidable ruler for frequency measurements Sep 16, 2025
@naglepuff naglepuff marked this pull request as ready for review September 16, 2025 18:02
Copy link
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

Everything functions correctly and the code looks fine to me. My questions/suggestions are mostly some usability things.

There is a bug when swapping between compressed/non-compressed where you don't seem to be able to click to drive until the user toggles the layer on/off again.

I'm wondering if Y-Scaling (shift+scroll wheel) should keep the line at the same position (frequency wise) instead of the KHz changing? If a user places it and wants more details about the pulse by stretching in the Y-Axis it may be nice for the measurement line to stay at the same frequency and just have everything stretch around it.

I added that other PR to give visual feedback of when you can click on the measurement line so there wasn't as much guessing if I was close enough to the line to grab and drag it.

@naglepuff
Copy link
Collaborator Author

@BryonLewis check out the two recent commits, which should address your feedback.

Copy link
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

Just one minor thing, but everything else looks good.

Copy link
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

👍

@naglepuff naglepuff merged commit df8e29d into main Sep 24, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants