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

[UI v2] feat: Start transition flow runs list form data table to cards #17238

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

devinvillarosa
Copy link
Contributor

@devinvillarosa devinvillarosa commented Feb 21, 2025

Start transition flow runs list form data table to cards

After discussion to use a card UX instead, this PR starts the transition to use cards. Future PRs will slowly build up the cards.
This PR copies over pagination features over from the data table UX

Screen.Recording.2025-02-21.at.10.45.13.AM.mov

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

Relates to #15512

@github-actions github-actions bot added the ui-replatform Related to the React UI rewrite label Feb 21, 2025
@devinvillarosa devinvillarosa marked this pull request as ready for review February 21, 2025 18:49
Copy link
Contributor

@pleek91 pleek91 left a comment

Choose a reason for hiding this comment

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

Few non blocking comments, overall LGTM!

Comment on lines +8 to +16
type FlowRunCardProps =
| {
flowRun: FlowRunRow;
}
| {
flowRun: FlowRunRow;
checked: boolean;
onCheckedChange: (checked: boolean) => void;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Comment on lines +3 to +22
export const FLOW_RUN_STATES = [
"Scheduled",
"Late",
"Resuming",
"AwaitingRetry",
"AwaitingConcurrencySlot",
"Pending",
"Paused",
"Suspended",
"Running",
"Retrying",
"Completed",
"Cached",
"Cancelled",
"Cancelling",
"Crashed",
"Failed",
"TimedOut",
] as const;
export type FlowRunState = (typeof FLOW_RUN_STATES)[number];
Copy link
Contributor

Choose a reason for hiding this comment

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

These are state names (vs state types) so lets be specific and call these FlowRunStateNames. Also, important to keep in mind that there can be user provided state names. So if we have the state type, we should work off that. And wherever we use state names we should type them as string rather than a specific union type like this (in components that accept customer runs).

This is helpful for mapping known state names to state types, but we cannot count on the state name being one of these known values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I'll keep that in mind.
I'll keep this for now until I update the zod validation on the routes for this search param

@devinvillarosa devinvillarosa merged commit 27e85f9 into main Feb 21, 2025
9 checks passed
@devinvillarosa devinvillarosa deleted the flow-runs-card-list branch February 21, 2025 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui-replatform Related to the React UI rewrite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants