[ui] Allow both horizontal and vertical renderings of the asset graph #19954
Conversation
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit e3091fd. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit e3091fd. |
js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNodeMenu.tsx
Show resolved
Hide resolved
|
I kind of want to right align the shortcuts in the context menu: |
|
Also I realize collapse/expand have the same shortcut 🤔 I wonder which one takes precedence if both are possible. Maybe we need different shortcuts or some way of indicating which would take precedence? |
js_modules/dagster-ui/packages/ui-core/src/assets/AssetNodeLineageGraph.tsx
Outdated
Show resolved
Hide resolved
|
Looks like in the mixed state you mentioned the shortcut performs "collapse all". I updated the code for the menu so the rendering reflects that:
|
…#19954) ## Summary & Motivation Back by popular demand! The vertical asset graph returns as an option accessible from a context menu on the asset graph, a small button in the bottom right, and a shortcut (Alt-O) <img width="636" alt="image" src="https://github.com/dagster-io/dagster/assets/1037212/86634bd1-881b-4548-b8e3-d6014b90e3ff"> <img width="640" alt="image" src="https://github.com/dagster-io/dagster/assets/1037212/d70bdb9f-777d-484a-a67d-1480320edd61"> <img width="671" alt="image" src="https://github.com/dagster-io/dagster/assets/1037212/291a87f3-eef2-4f7d-bbaf-55d24e63d1b6"> This PR also makes some incremental + related improvements: - A few icons have been resized to our standard 16x16 so the toolbar on the asset graph looks consistent. - The icons shown in Menu are now textDefault instead of accentGray (Verified this change with Josh) - I accidentally started putting the new context menu option in `AssetNodeContextMenu` before I realized I wanted it on the background context menu instead. In the process, I made a few code improvements there (shared type for the props, etc) that I left in the PR. - The edges on the graph now go OVER the group background but UNDER the group title bars, which helps a lot in vertical mode. - The borders around asset groups are a color that uses opacity, so that the "stack lines" beneath them darken when they collide with edges. - The op graph has switched back to the correct line styling. It was using horizontal joiners incorrectly. Before:  After:  ## How I Tested These Changes I referenced the storybooks for the asset node tweaks, but the rest of this I tested manually with a couple large graphs. --------- Co-authored-by: bengotow <[email protected]>
Summary & Motivation
Back by popular demand! The vertical asset graph returns as an option accessible from a context menu on the asset graph, a small button in the bottom right, and a shortcut (Alt-O)
This PR also makes some incremental + related improvements:
A few icons have been resized to our standard 16x16 so the toolbar on the asset graph looks consistent.
The icons shown in Menu are now textDefault instead of accentGray (Verified this change with Josh)
I accidentally started putting the new context menu option in
AssetNodeContextMenubefore I realized I wanted it on the background context menu instead. In the process, I made a few code improvements there (shared type for the props, etc) that I left in the PR.The edges on the graph now go OVER the group background but UNDER the group title bars, which helps a lot in vertical mode.
The borders around asset groups are a color that uses opacity, so that the "stack lines" beneath them darken when they collide with edges.
The op graph has switched back to the correct line styling. It was using horizontal joiners incorrectly.
Before:
After:
How I Tested These Changes
I referenced the storybooks for the asset node tweaks, but the rest of this I tested manually with a couple large graphs.