Page MenuHomePhabricator

Add support for composite solids to the Explore tab
ClosedPublic

Authored by bengotow on May 24 2019, 9:51 PM.

Details

Summary

This diff adds breadcrumbs to the explore tab and allows you to double-click into composite solids, which have a new purplish + double bordered appearance. When you're within a composite solid, the DAG shows a containing box indicating the solid you're within, and when you don't have a selection you can view the parent solid and its input/output mappings in the right sidebar.

Hovering over an input / output of a composite that is connected to one of it's child solids displays the name of the child. We should be able to do this the other way around as well. Will investigate that in a stacked diff.

This diff also changes the URL structure within the explore tab. The path segments are the handle IDs, and a trailing slash indicates you're within the last path component rather than just selecting it. It seems that the solid HandleIDs are generated as dot-separated paths, so there's a bit of duplication in the URL currently, eg:

http://localhost:3000/composition/explore/add_four/add_four.adder_1/add_four.adder_1.adder_1

I think we could fix this by allowing the client to rely on this HandleID structure, but I wasn't sure if this was a good idea.

(Note the purple arrows were removed!)

Test Plan

Updated the snapshot tests

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bengotow created this revision.May 24 2019, 9:51 PM
bengotow edited the summary of this revision. (Show Details)May 24 2019, 10:00 PM
bengotow added reviewers: alangenfeld, schrockn.
bengotow updated this revision to Diff 1019.May 24 2019, 10:04 PM
bengotow edited the summary of this revision. (Show Details)
  • Mark that @types/color is a devDependency
alangenfeld added inline comments.May 24 2019, 10:37 PM
js_modules/dagit/src/PipelineExplorer.tsx
85–88

there could be the same named solid in multiple places in the composition tree so this seems not right

js_modules/dagit/src/PipelineExplorerRoot.tsx
20

+8 whats up with this?

schrockn requested changes to this revision.May 24 2019, 11:01 PM

Ok so I have a few pieces of feedback here at a high level just in terms of interaction.

  • I think we should be able to zoom "out" of a composite with a double click. Namely if one double clicks anywhere in the "gray space" outside of where the solid is defined, let's pop up a level.
  • So one thing I find frustrating is that I have to double click twice in order to zoom. The sequence of actions is:
    1. Click one to select
    2. Double click to center/zoom
    3. Double click to pop down a level. It actually feels like a bug.

      Not sure what the exact right answer is here because changing this behavior would cause potential inconsistency in the non solid use case. We could make it so that a zoom in when you have a composite selected does that pop down.
  • We should add a subtle animation whenever we go up and down the stack. Doesn't need to be crazy but enough to know that something happened. The additional item in the breadcrumb isn't "enough" to communicate to the user that something as profound as a hop down to a composite has happened.

I actually think only the first point is important enough to block this merge. However I do think the the animation is strictly required before we demo this to anyone. I think the "many click" problem will be harder to resolve.

This revision now requires changes to proceed.May 24 2019, 11:01 PM
schrockn added inline comments.May 24 2019, 11:03 PM
js_modules/dagit/src/PipelineExplorerRoot.tsx
20

i assume it is len('explore/')

Hey folks! Sounds good—we are very far from being able to animate this transition, but I think we can hack something together that communicates what you're looking for. In terms of the zooming in I agree it's a lot of clicks, but I think it's worth considering how often the user will actually want to look inside the solids. I figured this was something you'd do infrequently, and clicking around and having it "jump" inside what you were looking at would actually be more annoying. Regardless, should be easy to come up with something else. There's also an "Expand" button in the sidebar, and we could just put an explicit button in the DAG view as well.

js_modules/dagit/src/PipelineExplorerRoot.tsx
20

Ahh yeah I'll fix this—I think it's actually "explore".length + 1 which is just a bit awkward.

schrockn accepted this revision.May 29 2019, 4:05 PM

Let's go ahead and land this and then work on the incremental improvements in follow on diffs.

This revision is now accepted and ready to land.May 29 2019, 4:05 PM
bengotow updated this revision to Diff 1278.May 31 2019, 7:03 PM
  • Switch from indexOf + length to split+pop
  • Add ability to double click out of a composite solid by clicking the dark gray background
  • Finally have a good solution for animation—CSS rect transition + react keys
  • Use the solid tags area to display an “Expand” button
bengotow updated this revision to Diff 1283.May 31 2019, 7:10 PM
  • Slow down the animation a bit
  • Click solid before expanding it to prepare the initial animation state
  • Move the label on the parent solid container inside to prevent rect bounds from shifting when you zoom
schrockn accepted this revision.May 31 2019, 8:14 PM

This is pretty fantastic. Excited to play with it.

bengotow updated this revision to Diff 1320.Jun 1 2019, 4:49 PM
  • Looks like BuildKite wants types rebuilt
bengotow updated this revision to Diff 1351.Jun 3 2019, 3:19 PM
  • Remove unneeded file modified every time generate-types runs
bengotow updated this revision to Diff 1352.Jun 3 2019, 4:03 PM
  • Mark that @types/color is a devDependency
  • Switch from indexOf + length to split+pop
  • Add ability to double click out of a composite solid by clicking the dark gray background
  • Finally have a good solution for animation—CSS rect transition + react keys
  • Use the solid tags area to display an “Expand” button
  • Slow down the animation a bit
  • Click solid before expanding it to prepare the initial animation state
  • Move the label on the parent solid container inside to prevent rect bounds from shifting when you zoom
  • Looks like BuildKite wants types rebuilt
  • Remove unneeded file modified every time generate-types runs
bengotow updated this revision to Diff 1374.Jun 3 2019, 7:22 PM
  • Remove unneeded file modified every time generate-types runs
  • Generate types using Python 3.7 to match BuildKite behavior