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/histogram slider #127

Merged
merged 7 commits into from
Nov 20, 2024
Merged

Feature/histogram slider #127

merged 7 commits into from
Nov 20, 2024

Conversation

jnation3406
Copy link
Contributor

This adds the HistogramSlider component and uses that for the image scaling controls. It depends on this backend PR.

histogram_scaling.webm

Copy link
Contributor

@LTDakin LTDakin left a comment

Choose a reason for hiding this comment

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

Looks good, just some file organization

thumb-label="always"
strict
hide-details
@update:modelValue="updateScaleRange"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need to change it, but wanted to check this to help me learn more about vue. A watch function on slider Range would be equivalent to listening for an update event of the components model Value. Only benefit is someone reading it that doesn't know vue as well may be able to understand watchSliderRange vs making the connection that the variable in v-model is what is triggering the @ update:modelValue event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there can actually be a difference in behavior between those two things in practice. Setting an on update handler in the component will only get triggered when the value changes due to the component, i.e. by moving the slider. This is different from a watcher, that will get triggered any time the value changes by any means, including just setting it elsewhere in the code. So you'll see I do slightly different logic based on if the value is changed from the range slider vs from the number inputs, even though both of those things will ultimately change the value in the variable.

Copy link
Contributor

Choose a reason for hiding this comment

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

ohhhh good to know actually, thanks for the explanation

src/components/Global/HistogramSlider.vue Outdated Show resolved Hide resolved
@LTDakin LTDakin merged commit d3008eb into main Nov 20, 2024
@LTDakin LTDakin deleted the feature/histogram_slider branch November 20, 2024 00:43
LTDakin added a commit that referenced this pull request Nov 20, 2024
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.

3 participants