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

Reorient view during relayout based on visible elements #163

Merged
merged 6 commits into from
Sep 3, 2024
Merged

Conversation

tbennun
Copy link
Contributor

@tbennun tbennun commented Sep 1, 2024

Relayout now tries to keep track of the visible elements from before relayouting. This should allow more fluid transition when collapsing elements. The heuristic condition is: every node that was entirely visible before the relayouting should still be visible after it. If they are all already visible, no changes are made to the view (we would like to minimize involuntary view changes as much as possible).
Additionally, if a specific element instigated the change (i.e., a node was collapsed), it will also be included in the nodes to view.

The PR also fixes a visual artifact introduced wherein collapsed nodes still appear for a few frames with their old coordinates, which results in a badly-drawn graph.

tbennun and others added 5 commits August 31, 2024 17:08
Relayout now tries to keep track of the visible elements from before relayouting. This should allow more fluid transition when collapsing elements.
@tbennun tbennun marked this pull request as ready for review September 1, 2024 02:51
Copy link
Collaborator

@phschaad phschaad left a comment

Choose a reason for hiding this comment

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

Cool feature, looks good to me - thanks!
One thing I am slightly unsure about is whether the behavior should be something you can toggle on/off. I am thinking about it from a standpoint of 'consistency', in the sense that we are doing something that a user did not directly do (i.e., moving and potentially zooming without explicit action from the user other than collapse/expand). Maybe a user should be able to disable that if they find it irritating/confusing - despite it probably helping most users? Thoughts?

@tbennun tbennun merged commit ca8608e into master Sep 3, 2024
2 checks passed
@tbennun tbennun deleted the reorient branch September 3, 2024 14:40
phschaad pushed a commit that referenced this pull request Sep 4, 2024
Relayout now tries to keep track of the visible elements from before relayouting. This should allow more fluid transition when collapsing elements. The heuristic condition is: every node that was entirely visible before the relayouting should still be visible after it. If they are all already visible, no changes are made to the view (we would like to minimize involuntary view changes as much as possible).
Additionally, if a specific element instigated the change (i.e., a node was collapsed), it will also be included in the nodes to view.

The PR also fixes a visual artifact introduced wherein collapsed nodes still appear for a few frames with their old coordinates, which results in a badly-drawn graph.
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