This patch package is pretty intriguing. I'm OK with doing this in this isolated instance because we're likely to abandon codemirror at some point in the future (for Monaco) and forking / fixing the library to make this change seems like overkill. I'm curious to get @dish 's thoughts + approval on this though because he investigated that change.

This looks good to me! I feel like it's a bit wonky to have the nice big solids list on the page, and the minimal solids list in the sidebar, but we can probably address that when we revisit that whole sidebar.

This looks great to me.

• Fix retina rendering of the sparklines
• Expose stepStats for related step, add elapsed time to charts / matrix

In D5147#140032, @schrockn wrote:

for whatever reason getting this error when running locally

Make it robust to users accidentally capturin the setter in scopes

Add tests!

This looks good to me! I'd be surprised if the top level color makes a difference in many places? I think all the blueprint input components should have higher-specificity color styles (I hope so, because there's a dark mode 😅) but it'll be nice to soften any hard blacks we have laying around!

Looks good to me 👍

Self-review, split components and rm GraphQL fragments that don't map to components

Oh this is cool - I didn't realize that there was a linter plugin for this.

This looks good to me! Just added a few comments inline.

Hey! I think this looks good. In the past I've been broadly skeptical of base components like this because I think learning how the custom DSL maps to CSS and then mentally unfolding it as you read the JSX isn't worth the cleanliness points of not writing style={display: flex, flexDirection: column} inline.

• Make sure that the component unmounting stops the last run of usePartitionQuery

Hey Sandy thanks for the feedback! I fixed a few of the issues you noticed but a few other things:

• Compact the top bar of the run matrix a bit
• Fix issue with matrix “showing data from runs outside the filter” when refreshing

Yep this seems like a great change! My only concern is that the blue for "Started" isn't consistent with the other places we currently use the old status dots (the Run Group sidebar on the Run details page and the Pipeline Overview page are the only two that come to mind), but we can replace those incrementally.

Remove RunGraph class, rename RunGraphUtils > PartitionGraphUtils to reflect the current state of the system, we will reconsider base class / common types for graphs but for now it's more important that the requested types reflect exactly the data we need.

• Fix merge issues

In D4898#133475, @sandyryza wrote:

I don't think I'm qualified to comment on the code, but I love the result!

I would push lightly against including the expectation count / materialization count. Just for the sake of reducing clutter. Though maybe I'm missing some use for them that you had in mind?

This looks good to me - I guess it replaces the MockedProvider we already have in place in App.test.tsx?

Nice! I think this is a good move 👍 Initially we went with this "cards" appearance for the table rows because the background was super light blue and not white (only the rows were white). Now that all the pages of the app have white backgrounds, the drop shadows and spacing are just adding whitespace.

This looks awesome! The components are super clean and I think the SearchResultType and switch statements will make it easy to extend this to new types of results which is great. This is also a very nice use of useReducer in here.

LGTM!

• Enable subset selector without previous runs

Ahh man, yeah I suppose this could use YAML. I personally prefer the ... syntax because it encodes that the range is inclusive of the end, but that's not hard to see from the live preview. We'll need to reflow the UI a bit because that'll take up a lot more vertical space.

Rebase!

This looks good to me!

Just added a couple nits, overall I think this is a huge improvement and we can incrementally improve the UI of each page as we collect feedback.

This looks great! Good catch 👍

Looks great!

Simplify state a bit - coerce into a valid state when you make changes so enabled/disabled state of various features don't need to reference many vars

Thanks for the review! Will add the subset w/o previous runs logic changes. Just needed to re-read Sandy's doc now 👍

• Apply diff feedback

Thanks for the feedback! Going to apply everything and then get this merged so I can stack the next round of changes on it.

• Remove old loader, address diff feedback, add new loading state to chunked query

Hey @sashank thanks for the careful review, I appreciate it! Going to address your comments and handle a couple of those edge cases better.

This looks great! I think the new breadcrumbs approach is a lot more clear. Just left a few inline comments but from a code / usability standpoint I think this is a win.

This looks great! Did not realize we had a useConfirmation hook, but that makes this super clean. (Though I assume it's stateless and doesn't only remind you on the removal of the first tab?)

This looks great to me! Just left a few inline comments but nothing requiring any action 👍

I think this is a great change—I know Prezi mentioned on the call last thursday that they click the pipeline name by accident too. I like the addition of the Historical tag and I think calling the column "Snapshot" makes it really clear that the underlying system is snapshotting your pipeline each time it's executed, which is cool and under-communicated in our old design. I think it'd be nice to make the click target a bit bigger by merging the run hash and the pipeline name into a single link but that's about it - big wins here!

Fix issue

This looks great! I added a few inline comments but I like the idea of creating a base class for these graphs, the chartJS stuff is pretty verbose and it's always a bummer to have a ton of chart config in the middle of a component.

LGTM! Thanks for the efforts to make this and buildkite faster, especially for JS-only changes it's been awesome.

This looks great! In the future it might be a good idea to merge this with RunStatusToPageAttributes, which is responsible for turning the Dagit favicon green / red / gray based on the status of the current run.

This looks really solid! I think the consistency across schedules / assets is a huge win and this ended up being less code than I expected too! 👍

Constify FLAT_INSET_FROM_PARENT

• Import ChartJS types

• Import ChartJS types

• Pre-rebase changes
• Merge branch 'master' into dbg/one-click-2
• Resolve merge conflicts

Thanks for the feedback folks! I definitely agree Yuhan re: future improvement 👍

Rebase