-
Notifications
You must be signed in to change notification settings - Fork 205
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] Use tabbed pane with Queries and Executors tabs #309
Conversation
@KenSuenobu @onthebridgetonowhere Could you review this PR when you get a chance? |
ballista/ui/scheduler/src/App.tsx
Outdated
return ( | ||
<Box> | ||
<Grid minH="100vh"> | ||
<VStack alignItems={"flex-start"} spacing={0} width={"100%"}> | ||
<Header schedulerState={schedulerState} /> | ||
<Summary schedulerState={schedulerState} /> | ||
<QueriesList queries={jobs} /> | ||
<Tabs> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably want to have here: <Tabs width={"100%"}>
}) => { | ||
return ( | ||
<Box flex={1}> | ||
<DataTable maxW={960} columns={columns} data={nodes} pageSize={4} /> | ||
<DataTable maxW={960} columns={columns} data={executors} pageSize={4} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maxW
property is not really useful, unless we need this width size restriction. Removing it makes the table look better.
@andygrove it looks great. I have only two comments regarding look&feel, see above. |
Thanks for the review @onthebridgetonowhere. I addressed those two points and it looks much better now. |
Which issue does this PR close?
N/A
Rationale for this change
The list of executors could be very large so it does not make sense to show in the header.
What changes are included in this PR?
Jobs Tab
Executors Tab
Are there any user-facing changes?