[ui] Implement new asset Automation page #26765
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 710632a. |
hellendag
force-pushed
the
dish/automation-changes
branch
from
a486dcf to
8654307
Compare
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 710632a. |
hellendag
changed the title
| @@ -4,6 +4,7 @@ import {Button} from './Button'; | |||
| import {Icon} from './Icon'; | |||
|
|
|||
| export interface CursorPaginationProps { | |||
| cursor?: string; | |||
There was a problem hiding this comment.
Added this so that the parent component can determine when the cursor has changed, and reset scroll position accordingly.
There was a problem hiding this comment.
Ahh I think I did this on the new RunsFeed page as well, but a bit differently. That version watches for the state to transition to loading and then back to ready (RunsFeedTable.tsx::84), but maybe that should be switched to this more direct approach too..
I think one issue I was running in to was that it looks a bit janky to scroll to the top before the data has loaded -- watching the state transition was the best way I could find to scroll only once the new data was available.
I wonder if we could have useCursorPaginatedQuery return the cursor that matches the data it loaded, and not the cursor that is is is loading, so that it behaves that way?
js_modules/dagster-ui/packages/ui-core/src/assets/AssetView.tsx
Outdated
Show resolved
Hide resolved
| zIndex: 1, | ||
| boxShadow: `inset 0 -1px 0 ${Colors.keylineDefault()}`, | ||
| }} | ||
| > |
There was a problem hiding this comment.
Sticky header while scrolling.
There was a problem hiding this comment.
I wonder if we should go ahead and switch this out for a virtualized list? We have the HeaderRow for that one... With the row component this table already feels pretty close to our standard virtualized one.
<HeaderRow templateColumns={TEMPLATE_COLUMNS} sticky>
There was a problem hiding this comment.
It probably could be virtualized, it just seemed a bit overkill for a table with a maximum of 25-30 items.
hellendag
force-pushed
the
dish/automation-changes
branch
from
8654307 to
958bf49
Compare
...les/dagster-ui/packages/ui-core/src/assets/AutoMaterializePolicyPage/AssetAutomationRoot.tsx
Outdated
Show resolved
Hide resolved
...dules/dagster-ui/packages/ui-core/src/assets/AutoMaterializePolicyPage/EvaluationListRow.tsx
Show resolved
Hide resolved
hellendag
force-pushed
the
dish/automation-changes
branch
from
958bf49 to
a56285f
Compare
| @@ -4,6 +4,7 @@ import {Button} from './Button'; | |||
| import {Icon} from './Icon'; | |||
|
|
|||
| export interface CursorPaginationProps { | |||
| cursor?: string; | |||
There was a problem hiding this comment.
Ahh I think I did this on the new RunsFeed page as well, but a bit differently. That version watches for the state to transition to loading and then back to ready (RunsFeedTable.tsx::84), but maybe that should be switched to this more direct approach too..
I think one issue I was running in to was that it looks a bit janky to scroll to the top before the data has loaded -- watching the state transition was the best way I could find to scroll only once the new data was available.
I wonder if we could have useCursorPaginatedQuery return the cursor that matches the data it loaded, and not the cursor that is is is loading, so that it behaves that way?
| zIndex: 1, | ||
| boxShadow: `inset 0 -1px 0 ${Colors.keylineDefault()}`, | ||
| }} | ||
| > |
There was a problem hiding this comment.
I wonder if we should go ahead and switch this out for a virtualized list? We have the HeaderRow for that one... With the row component this table already feels pretty close to our standard virtualized one.
<HeaderRow templateColumns={TEMPLATE_COLUMNS} sticky>
Merge activity
|
hellendag
force-pushed
the
dish/automation-changes
branch
2 times, most recently
from
0b347b6 to
c6a3416
Compare
hellendag
force-pushed
the
dish/automation-changes
branch
from
c6a3416 to
710632a
Compare
Summary & Motivation
Update the Automation tab on assets to bring the page closer to @braunjj's design changes. Some polish is likely to be needed.
This change also includes @OwenKephart's #26170, which ended up needing a handful of changes to make the evaluations table work for both assets and asset checks.
Non-partitioned asset:
Partitioned asset:
How I Tested These Changes
View assets that use automation conditions. Verify that the conditions and evaluation info dialog render correctly on the asset page.
Changelog
[ui] Update asset Automations tab.