A first pass at flattening the left nav to display jobs, with iconography to indicate schedule/sensor status, as discussed.
Details
Details
- Reviewers
• bengotow - Commits
- R1:213ae6d7e406: [dagit] Crag: Flattened left nav jobs
Enable flag, verify that jobs appear in the left nav, with icons.
Diff Detail
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. | |
Comment Actions
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 👍 | |
Comment Actions
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. | |