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

Allow changing plot aspect ratio with scroll + cmd/ctrl + alt #2742

Merged
merged 1 commit into from
Aug 6, 2023

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jul 18, 2023

What

Wanted Shift+Cmd originally, but egui no longer see this as a scroll then. Also, shift+scroll is already taken for translating along y.

Fixes #2148

Screen.Recording.2023-07-18.at.16.06.29.mov

Checklist

@Wumpf Wumpf added enhancement New feature or request 📺 re_viewer affects re_viewer itself labels Jul 18, 2023
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

I generally agree with it but the logic seems flipped right now? When I go to the web demo, scroll + ctrl + alt does an XY zoom, while scroll + ctrl does a X-only zoom.

@Wumpf
Copy link
Member Author

Wumpf commented Jul 19, 2023

wait what? It's the opposite for me. When I go scroll + cmd I get X and Y zoomed in equally. Tried in Firefox and Chrome. Let's quickly follow up after daily call

@Wumpf
Copy link
Member Author

Wumpf commented Jul 19, 2023

seems to be another instance of sticky modifiers on the web T_T

@Wumpf Wumpf added the do-not-merge Do not merge this PR label Jul 25, 2023
@teh-cmc
Copy link
Member

teh-cmc commented Aug 2, 2023

seems to be another instance of sticky modifiers on the web T_T

Yeah actually I can confirm that CTRL has buggy sticky behavior on web, even outside the context of this PR.

Sometimes the web viewer is just convinced that I'm pressing CTRL for seemingly no reason 🤷‍♂️

@Wumpf
Copy link
Member Author

Wumpf commented Aug 2, 2023

@teh-cmc so.. let's treat this as orthogonal and land this aging PR anyways? 🤔 On the flipside it makes sticky modifier problem worse

@teh-cmc
Copy link
Member

teh-cmc commented Aug 3, 2023

I don't mind either way.

In my case it makes the UX worse on Firefox but better on Chromium so maybe that's fine 🤷

@Wumpf Wumpf removed the do-not-merge Do not merge this PR label Aug 6, 2023
@Wumpf
Copy link
Member Author

Wumpf commented Aug 6, 2023

more users asking for this, let's merge it and tackle the browser issue separately

@Wumpf Wumpf merged commit ac8bc41 into main Aug 6, 2023
@Wumpf Wumpf deleted the andreas/single-axis-plot-zoom branch August 6, 2023 06:49
@Wumpf Wumpf added the user-request This is a pressing issue for one of our users label Aug 6, 2023
@Wumpf Wumpf added this to the 0.8.1 milestone Aug 8, 2023
emilk pushed a commit that referenced this pull request Aug 17, 2023
<!--
Open the PR up as a draft until you feel it is ready for a proper
review.

Do not make PR:s from your own `main` branch, as that makes it difficult
for reviewers to add their own fixes.

Add any improvements to the branch as new commits to make it easier for
reviewers to follow the progress. All commits will be squashed to a
single commit once the PR is merged into `main`.

Make sure you mention any issues that this PR closes in the description,
as well as any other related issues.

To get an auto-generated PR description you can put "copilot:summary" or
"copilot:walkthrough" anywhere.
-->

### What

Wanted Shift+Cmd originally, but egui no longer see this as a scroll
then. Also, shift+scroll is already taken for translating along y.

Fixes #2148
* #2148



https://github.com/rerun-io/rerun/assets/1220815/bc078094-4bae-4980-8508-af0cff234614



### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2742) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2742)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fsingle-axis-plot-zoom/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fsingle-axis-plot-zoom/examples)
@emilk emilk mentioned this pull request Aug 17, 2023
3 tasks
emilk pushed a commit that referenced this pull request Aug 17, 2023
<!--
Open the PR up as a draft until you feel it is ready for a proper
review.

Do not make PR:s from your own `main` branch, as that makes it difficult
for reviewers to add their own fixes.

Add any improvements to the branch as new commits to make it easier for
reviewers to follow the progress. All commits will be squashed to a
single commit once the PR is merged into `main`.

Make sure you mention any issues that this PR closes in the description,
as well as any other related issues.

To get an auto-generated PR description you can put "copilot:summary" or
"copilot:walkthrough" anywhere.
-->

### What

Wanted Shift+Cmd originally, but egui no longer see this as a scroll
then. Also, shift+scroll is already taken for translating along y.

Fixes #2148
* #2148



https://github.com/rerun-io/rerun/assets/1220815/bc078094-4bae-4980-8508-af0cff234614



### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2742) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2742)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fsingle-axis-plot-zoom/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fsingle-axis-plot-zoom/examples)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 📺 re_viewer affects re_viewer itself user-request This is a pressing issue for one of our users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make it easy to zoom in on only a single axis in a plot
2 participants