Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2/k][ui] Replace compute kind filters with kind filters #24160

Merged
merged 7 commits into from
Sep 9, 2024

Conversation

benpankow
Copy link
Member

@benpankow benpankow commented Sep 2, 2024

Summary

Re-implementation of #23805 which is slightly lighter weight (but still touches a lot of surface area). Substitutes in the notion of a generic kind filter rather than specifically compute kind.

Test Plan

Existing unit tests, local dagit.

Changelog

NOCHANGELOG

Copy link

github-actions bot commented Sep 2, 2024

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-3f2833vfd-elementl.vercel.app
https://benpankow-kinds-ui-2.core-storybook.dagster-docs.io

Built with commit 0e0981d.
This pull request is being automatically deployed with vercel-action

@benpankow benpankow force-pushed the benpankow/kinds-ui-1 branch from 8dcf5cd to 6e6b2eb Compare September 4, 2024 14:54
@benpankow benpankow force-pushed the benpankow/kinds-ui-2 branch from aa198f9 to 05dc5fe Compare September 4, 2024 14:54
@benpankow benpankow changed the base branch from benpankow/kinds-ui-1 to graphite-base/24160 September 4, 2024 21:53
@benpankow benpankow force-pushed the benpankow/kinds-ui-2 branch from 05dc5fe to 37e8e7a Compare September 4, 2024 21:56
@benpankow benpankow changed the base branch from graphite-base/24160 to master September 4, 2024 21:57
@benpankow benpankow force-pushed the benpankow/kinds-ui-2 branch 2 times, most recently from dc2569d to 84f0c61 Compare September 4, 2024 22:16
@benpankow benpankow marked this pull request as ready for review September 4, 2024 22:31
@benpankow benpankow requested a review from salazarm September 4, 2024 22:31
Comment on lines 78 to 100

export function useAssetTagsForAssets(
assets: {definition?: {tags?: DefinitionTag[] | null} | null}[],
): DefinitionTag[] {
return useMemo(
() =>
Array.from(
new Set(
assets
.flatMap((a) => a.definition?.tags?.map((tag) => JSON.stringify(tag)) ?? [])
.filter((o) => o),
),
)
.map((jsonTag) => buildDefinitionTag(JSON.parse(jsonTag)))
.sort((a, b) =>
// Sort by key then by value
a.key.localeCompare(b.key) === 0
? a.value.localeCompare(b.value)
: a.key.localeCompare(b.key),
),
[assets],
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any changes here?

@@ -152,7 +152,6 @@ export function useAssetCatalogFiltering<
[
['owners', filters.owners, allAssetOwners] as const,
['tags', filters.tags, tags] as const,
['kinds', filters.kinds, allKinds] as const,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we remove this?

@benpankow benpankow force-pushed the benpankow/kinds-ui-2 branch from f803e74 to 0e0981d Compare September 6, 2024 22:38
@benpankow benpankow merged commit fafef8a into master Sep 9, 2024
2 checks passed
@benpankow benpankow deleted the benpankow/kinds-ui-2 branch September 9, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants