Page MenuHomeElementl

[dagit] Consistent top nav
ClosedPublic

Authored by dish on Feb 8 2021, 10:49 PM.

Details

Summary

Produce a consistent top navigation experience across Dagit, in advance of left nav changes.

  • Use Heading to clarify the object (or list, view, etc.) being viewed.
  • Use a "description" line beneath the heading to indicate a brief summary of the object, e.g. the type of object and its home repository
    • Use links in the description, e.g. "Pipeline in foo@bar" would link "Pipeline" to the full pipelines list and "foo@bar" to the repository view
  • On Run root, move metadata table to be adjacent to the heading and description, reclaiming some space
  • On Assets, continue to use breadcrumbs to link between asset paths

This eliminates the existing gray TopNav component and its associated breadcrumbs, as users will now ideally be able to use the "description" links to browse through Dagit.

As a followup, I'll want to change some spacing on some of the main sections of these pages. Fairly minor tweaks though.

Screenshots below.

Test Plan

View pages throughout Dagit. Verify that top sections render properly, with correctly positioned elements. Verify that links and other controls work as expected.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dish requested review of this revision.Feb 8 2021, 10:55 PM

This looks great! I like this direction and the consistency of the header + subtitle approach a lot. Just have a couple drive-by comments below, but I think we should merge this ASAP.

  • I'm a little wary of the run header taking up so much space - the rest of the page has a much higher information density than the header now. It looks nice though - if anything, maybe we make all of these headers a bit smaller / less playful if the team feels the same way.
  • The pipeline snapshot and run pages (screenshots 6 and 7) look very similar since the header is just an object ID, and the green/red state tags are nice but talking about different aspects of state. I like the icon corresponding to the object type on the second line where you have it, so I don't really have any actionable suggestion. Just seems like it'd be nice to differentiate these a bit more somehow. I wonder if we should consider adopting prefixed IDs system wide to disambiguate things a bit - R-1ab21 is a run, S-1b1abc is a snapshot, etc?

Code looks great! Happy to have <PageHeader /> unifiying our wrappers.

This revision is now accepted and ready to land.Feb 9 2021, 6:08 PM

I'm a little wary of the run header taking up so much space - the rest of the page has a much higher information density than the header now. It looks nice though - if anything, maybe we make all of these headers a bit smaller / less playful if the team feels the same way.

Yeah, I was trying to be cautious with this, and it turns out it's a difference of only 8 pixels or so. The gray header already occupies a decent chunk of space. I was also wondering if there might be other top-level information that could belong in the header. I buried tags and config behind a single button, but we could possibly make more use of that space.

I'm open to adjusting these as needed, especially font-size of Heading, if it seems like it's taking up too much space. But I would prefer that we maintain spacing and type consistency, even for full-viewport sections like this one, if we can.

Just seems like it'd be nice to differentiate these a bit more somehow. I wonder if we should consider adopting prefixed IDs system wide to disambiguate things a bit - R-1ab21 is a run, S-1b1abc is a snapshot, etc?

Hmm, I could add a little redundancy here, like "Run 12c34e14" or "Snapshot 4b563a46" as the Heading.

Reduce RunRoot header by 8px to keep it equivalent to master height. Seems ok to me. Also changed some of the description links to stay repo-specific for now.

This revision was automatically updated to reflect the committed changes.