Page MenuHomeElementl

[dagit] Crag: Flattened left nav jobs
ClosedPublic

Authored by dish on Jun 14 2021, 7:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jan 7, 12:18 PM
Unknown Object (File)
Dec 19 2022, 7:39 AM
Unknown Object (File)
Dec 4 2022, 7:19 PM
Unknown Object (File)
Nov 17 2022, 11:55 PM
Unknown Object (File)
Nov 17 2022, 11:55 PM
Unknown Object (File)
Nov 17 2022, 11:55 PM
Unknown Object (File)
Nov 17 2022, 11:54 PM
Unknown Object (File)
Nov 17 2022, 11:54 PM
Subscribers
None

Details

Summary

A first pass at flattening the left nav to display jobs, with iconography to indicate schedule/sensor status, as discussed.

Test Plan

Enable flag, verify that jobs appear in the left nav, with icons.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

js_modules/dagit/packages/core/src/nav/LeftNav.tsx
76–85

Forgot that I had this in the diff too. Not sure if we want it yet, or if it should be flagged.

This looks great! I think the iconography + colors are nice and I'm a fan of not having the icons click through to the sensor / schedule pages. Ideally we'll just lift the valuable things off those pages onto the Job overview.

js_modules/dagit/packages/core/src/nav/FlatContentList.tsx
100

Terribly minor nit / personal preference, but the triple reduce and all the splatting here might be better expressed as a good old fashioned array.push inside of three nested for loops 😅

104

It might be good to double check that this div doesn't need the height from line 107 for the rest of the sidebar to render properly?

js_modules/dagit/packages/core/src/nav/LeftNav.tsx
76–85

Honestly I think this is a great immediate add, don't think it needs to be behind a flag 👍

This revision is now accepted and ready to land.Jun 14 2021, 9:11 PM

Ideally we'll just lift the valuable things off those pages onto the Job overview.

Yep, exactly.

js_modules/dagit/packages/core/src/nav/FlatContentList.tsx
100

Haha yeah, I actually started out with for-loops and decided to switch back to this. I'll spend a few minutes switching it back again to see how it looks, now that the important parts of the logic are in place.

+bengotow. Checked the empty state, height looks fine.

dish marked 2 inline comments as done.

One more minor tweak

This revision was automatically updated to reflect the committed changes.