Make an object a place a tab goes to - #2947
Merged
Merged
Conversation
The window kept where it was in two coordinates — which screen it was on, and a path the treemap had zoomed along — set from a dozen places. A click on a rectangle, a row of a list, a field in the details panel and a step of the chain each set them slightly differently, so which panes agreed with each other depended on which of the four had been used. Now a tab is one `Place`: an object, the pile of smaller objects left out of one, the object list with its filter, the leaks, the starred. The map is laid out at the object, the chain pane walks up to a GC root from it, the details panel summarises it, the tab is named after it. `Place.of(cell)` for a rectangle and `Place.Object(id)` everywhere else, through one `open`, so every way to an object is the same move. The stored path could go because it was never independent: in a dominator tree the way down to an object is unique, so the map root is a function of the object. `TreemapNavigation` is gone with it, and the map is rooted at the object clicked rather than zoomed along a chain to it — an object that dominates nothing is one rectangle of its own bytes, which is the honest answer to what it holds. Which tab a click means is the click's own answer, in `OpenIn`: a plain click moves the tab being read, a middle click, a ⌘/Ctrl click or "Open in a new tab" opens one behind it, and the buttons on the bar always open one in front. Every tab closes, the last one included, which leaves the heap dump read and the bar one click from a tab again. Each of the three panes folds away and the outer two drag wider.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reworks navigation and information hierarchy in Shark Explorer: an object is a first class thing you go to, and everything the window draws follows from which object a tab is on.
The problem
The window kept where it was in two coordinates — which screen it was on, and a path the treemap had zoomed along — set from a dozen places. A click on a rectangle, a row of the object list, a field in the details panel and a step of the chain each set them slightly differently, so which panes agreed with each other depended on which of the four you had used.
A tab is one
PlacePlace(shark-explorer-core) is the whole of where a tab is: an object, the pile of smaller objects left out of one, the object list with its filter, the leaks, the starred. Every pane is a function of it — the map is laid out atplace.viewRootObjectId, the chain walks up to a GC root from that object, the details panel summarises it, the tab is named after it.Place.of(cell)for a rectangle andPlace.Object(id)everywhere else, through oneopen, so every way to an object is the same move.The stored path could go because it was never independent: in a dominator tree the way down to an object is unique, so the map root is a function of the object.
TreemapNavigationandExplorerScreenare gone with it. The map is rooted at the object clicked rather than zoomed along a chain to it, so an object that dominates nothing draws as one rectangle of its own bytes — the honest answer to what it holds. How it is held is the chain pane's answer.Tabs
Which tab a click means is the click's own answer, decided in
OpenInand nowhere else:Open in a new tabTwo object lists filtered differently are two useful tabs, which is why the bar never reuses the tab of the same name. Every tab closes, the last one included — a window with no tab still holds the heap dump it spent seconds reading, and the bar is one click from a tab again. A tab's history is its own, so the back arrow never walks out of the tab you are reading. A tab is named class-plus-address, since a strip of a dozen instances of one class is only one you can pick out of if each tab says which instance it is.
The three panes are resizable against each other and each folds away to the button that unfolds it.
All objectsis nowObject list.Roles, which is also what makes this testable
Three things in the window can say
Whole heap dumpat once — the button on the bar, the tab, and the top row of every chain — and they are three different moves.Role.Buttonon the bar,Role.Tabon the strip, and no role at all on a row that navigates, which is a link rather than a button. Correct for a screen reader, and what lets an assertion say which of the three it means.Tests
PlaceandTabsare plain immutable state inshark-explorer-core, unit tested (TabsTest, 14 cases). NewTabStripTestcovers the tab rules end to end in the UI;ObjectsScreenTestholds the object list tests moved out ofExplorerAppTest. 175 UI tests,checkanddetektgreen.Notes and docs updated in the same change:
notes/decisions.md,notes/treemap-rendering.md,shark/shark-explorer/AGENTS.mdanddocs/shark-explorer.md.No change log entry — Shark Explorer's
Unreleasedis still✨ Initial release.