[ui] Some backfill page cleanup #16310
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 4dbeec5. |
b0460b6
to
0d3130a
Compare
| import {BackfillTableFragment} from './types/BackfillTable.types'; | ||
|
|
||
| const COLLATOR = new Intl.Collator(navigator.language, {sensitivity: 'base', numeric: true}); |
There was a problem hiding this comment.
Oh this is interesting, I haven't used this API before!
| @@ -41,36 +41,17 @@ export const AssetKeyTagCollection: React.FC<{ | |||
| style={{minWidth: '400px', maxWidth: '80vw', maxHeight: '70vh'}} | |||
| isOpen={showMore} | |||
| > | |||
| {showMore ? ( | |||
There was a problem hiding this comment.
Hmm I can't remember offhand if Dialog renders it's children or not when it's closed -- It might be nice to verify that we're not rendering the list content unless it's open. ( I think this tag collection appears in the run list so there can be a bunch of them on the page?)
There was a problem hiding this comment.
Yeah, the contents should not be rendered until the dialog is open. This is the default behavior of the underlying Blueprint dialog component.
There was a problem hiding this comment.
Ahh nice that's good! So maybe this statement didn't really do anything before...
| </DialogBody> | ||
| <div style={{height: '340px', overflow: 'hidden'}}> | ||
| <VirtualizedItemListForDialog | ||
| items={[...(backfill?.partitionNames || [])].sort((a, b) => COLLATOR.compare(a, b))} |
There was a problem hiding this comment.
This looks good to me! I commented down below about Dialog but it might be nice to throw an "if open, items={...}, else items={[]}" here. I'm not sure whether this requested dialog exists in any list views, but the partitionNames list can be 100k+ items so it'd be nice not to do the sorting until the list is onscreen.
There was a problem hiding this comment.
Ah ok, I'll at least bury the sort under another component.
0d3130a
to
4dbeec5
Compare
## Summary & Motivation A handful of changes and cleanup items on the backfill table and backfill page. - Virtualize the partition and asset key tables from these pages, reusing the component created for auto-materialization. It's just a generic virtualized item list anyway, so we can go ahead and reuse it here. (I moved and renamed the component to give it generic characteristics.) - When linking to an asset from the backfill page where a backfill is for a range, link to that default range on the asset page itself. - When a backfill header just shows the names of partitions that have been backfilled (with no dialog) use a `Tag` to give the partition names some separation. It's not perfect, but it seemed a little weird to just stick a comma between them. - Some alignment tweaks. <img width="557" alt="Screenshot 2023-09-05 at 9 51 06 AM" src="https://github.com/dagster-io/dagster/assets/2823852/e70676ab-e89b-4b36-89d2-8ad2fe337dc2"> <img width="580" alt="Screenshot 2023-09-05 at 9 50 58 AM" src="https://github.com/dagster-io/dagster/assets/2823852/c995a16f-7345-4a9f-a22e-2031e269b661"> ## How I Tested These Changes View Backfill page. Click to view assets affected by a backfill, verify dialog with virtualized contents. Verify same when clicking the link to see the affected partitions. Click into a backfill, verify same dialog and virtualization behavior when looking at the list of affected partitions. Click into a backfill that uses a partition range. Click to the asset, verify that the range is applied to the querystring of the URL and loads as the selected range on the asset page.
Summary & Motivation
A handful of changes and cleanup items on the backfill table and backfill page.
Tagto give the partition names some separation. It's not perfect, but it seemed a little weird to just stick a comma between them.How I Tested These Changes
View Backfill page. Click to view assets affected by a backfill, verify dialog with virtualized contents. Verify same when clicking the link to see the affected partitions.
Click into a backfill, verify same dialog and virtualization behavior when looking at the list of affected partitions.
Click into a backfill that uses a partition range. Click to the asset, verify that the range is applied to the querystring of the URL and loads as the selected range on the asset page.