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/441 "focus on this subtree" #480

Merged
merged 66 commits into from
Oct 31, 2021
Merged

Feature/441 "focus on this subtree" #480

merged 66 commits into from
Oct 31, 2021

Conversation

eh-am
Copy link
Collaborator

@eh-am eh-am commented Oct 27, 2021

This PR introduces new functionality called focus on this subtree, based on #441

1 - It allows to change the flamegraph root:
Peek 2021-10-29 16-02

Currently the state is tored locally, but in the future people could use to share a specific subtree.

2 - It still allows zooming
Peek 2021-10-29 16-04

3 - Clicking on the same node undoes the zoom:
Peek 2021-10-29 16-05

4 - You can mix and match zoom/focus, here for example we zoom and then we focus:
Peek 2021-10-29 16-06

This currently is scoped under a contextmenu event, up next this feature would be available under a toolbar as well.

@Rperry2174
Copy link
Contributor

Nice work! Looks awesome so far... I think for the text "Focus on this node" is less clear than "collapse" or "collapse above". Mostly because focus is a little more broad in that it could be interpreted multiple different ways in this context whereas collapse is a specific kind of focus.

@eh-am eh-am mentioned this pull request Oct 28, 2021
@eh-am eh-am marked this pull request as ready for review October 29, 2021 20:17
@eh-am eh-am changed the title Feature/441 Feature/441 "focus on this subtree" Oct 29, 2021
@eh-am eh-am mentioned this pull request Oct 30, 2021
Copy link
Contributor

@Rperry2174 Rperry2174 left a comment

Choose a reason for hiding this comment

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

This is awesome! Everything seems to be running pretty smoothly and couldn't find any edge cases thanks for adding the double-click functionality too!

@Rperry2174
Copy link
Contributor

Two potential ideas for how we can let people know about the right click functionality:

  1. We could put one of these: ⓘ somewhere where when you hover over it it says instructions about how right click adds functionality
  2. We could add it to the shortcuts list
    image

Up to you do I'd be fine merging this and thinking more about this part later too

@eh-am
Copy link
Collaborator Author

eh-am commented Oct 31, 2021

@Rperry2174 Great ideas! Let me merge this and do that work in a separate PR.

@eh-am eh-am closed this Oct 31, 2021
@eh-am eh-am reopened this Oct 31, 2021
@github-actions github-actions bot requested a review from Rperry2174 October 31, 2021 14:50
@eh-am eh-am merged commit cfe0ae8 into main Oct 31, 2021
@eh-am eh-am deleted the feature/441 branch October 31, 2021 14:54
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.

4 participants