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

Add missing screenshots of graph view archetypes #8371

Merged
merged 14 commits into from
Dec 11, 2024

Conversation

grtlr
Copy link
Contributor

@grtlr grtlr commented Dec 9, 2024

Related

What

This fixes many problems around the state of the graph view. Turns out the scene rect alone is not sufficient to store all of the viewer state (who would have thought 😇).

@grtlr grtlr added the exclude from changelog PRs with this won't show up in CHANGELOG.md label Dec 9, 2024
Copy link

github-actions bot commented Dec 9, 2024

Latest documentation preview deployed successfully.

Result Commit Link
fe31ac7 https://landing-okmm1hbf8-rerun.vercel.app/docs

Note: This comment is updated whenever you push a commit.

Copy link

github-actions bot commented Dec 10, 2024

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
fe31ac7 https://rerun.io/viewer/pr/8371 +nightly +main

Note: This comment is updated whenever you push a commit.

@grtlr grtlr requested a review from abey79 December 10, 2024 08:39
@grtlr grtlr marked this pull request as ready for review December 10, 2024 08:40
Copy link
Member

@abey79 abey79 left a comment

Choose a reason for hiding this comment

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

I'm not fan of the new behaviour, but I'm a bit torn on the appropriate way to go about it.

At the very least, resizing the view should maintain the center point of the view (e.g. if the view is centred on a graph, resizing the view keeps the view centred on that graph).

I general, I also tend to prefer that resizing the view also adjust the scale factor to maintain the current "field of view". However our scaling is not the greatest, and it's cap to zoom level 1. So maybe we dont do that part.

@grtlr
Copy link
Contributor Author

grtlr commented Dec 10, 2024

Before implementing this, I looked at how other canvas-like applications (Figma Web, Affinity Designer 2) handle resizing and copied that because that's probably what most users expect.

It think we can make case for both sides, but I wonder if we should stray to far from what others are doing?

@abey79
Copy link
Member

abey79 commented Dec 10, 2024

Before implementing this, I looked at how other canvas-like applications (Figma Web, Affinity Designer 2) handle resizing and copied that because that's probably what most users expect.

It think we can make case for both sides, but I wonder if we should stray to far from what others are doing?

I've check our other views. All have the "keep centring" behaviour, but none do any zooming on resize. We should stick to that exact same behaviour.

Copy link
Member

@abey79 abey79 left a comment

Choose a reason for hiding this comment

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

Thanks!

@grtlr grtlr merged commit c739dd8 into main Dec 11, 2024
37 checks passed
@grtlr grtlr deleted the grtlr/graph-examples-and-doc branch December 11, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
3 participants