-
Notifications
You must be signed in to change notification settings - Fork 43
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
✨ Add application tasks status icon and popover to application table #1985
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1985 +/- ##
==========================================
+ Coverage 39.20% 42.32% +3.12%
==========================================
Files 146 170 +24
Lines 4857 5434 +577
Branches 1164 1364 +200
==========================================
+ Hits 1904 2300 +396
- Misses 2939 3017 +78
- Partials 14 117 +103
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
client/src/app/pages/applications/applications-table/applications-table.tsx
Show resolved
Hide resolved
client/src/app/pages/applications/applications-table/applications-table.tsx
Show resolved
Hide resolved
client/src/app/pages/applications/applications-table/components/column-application-name.tsx
Show resolved
Hide resolved
client/src/app/pages/applications/applications-table/useDecoratedApplications.ts
Show resolved
Hide resolved
Note: Once konveyor/tackle-ui-tests#1150 is merged, CI will pass again. The problem fixed isn't from this PR. |
Created #1988 for the base refactoring that happend in the first few commits of this PR. It'll make this PR much easier to review. |
There is another e2e ui test fail in spec |
Includes: - Basic refactoring of applications-table.tsx - Add radash as a dependency for future use Pre-work for #1985 --------- Signed-off-by: Scott J Dickerson <[email protected]>
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.
I just want to add my approval that I have tested the UI changes in my minikube instance with latest images and can verify the functionality.
Assuming tests are passing I am good to merge
Align `TaskStateIcon` Icon status fields with `IconedStatus`. This amounts to setting some icons blue instead of black consistently across tables and the drawer. Related to #1985 Signed-off-by: Scott J Dickerson <[email protected]>
On the application table's name column, add a status icon. The icon summarizes the status of the various kinds of tasks attached to an application (a single application has 0 or more tasks while a task acts on a single application). The icon will be driven off looking at the most recent task (by `createTime`) per `kind` for an application: - If there are no tasks, show the application in No Tasks state - Else if any are "running", show the application in Running - Else if any are "queued", show the application in Queued state - Else if any are "failed", show the application as Failed - Else if any are "canceled", show the application as Canceled - Else if any are "succeeded", show the application as Success A popover on hover will render on the application name column to display summary information about the application's tasks. The popover will have a table that shows the most recent tasks for each kind of task associated to the application. - Task id - Task status icon with the task kind - How long ago the task was started/created (exact time in a tooltip) Changes: - Created `useDecoratedApplications()` hook to run all calculations and cross-referencing necessary to render most of the table. - Created `ColumnApplicationName` to handle the application name column with the status icon and popover. - Refactored `useFetchTasks()` hook to only do a simple sort. The filtering based on kind is no longer necessary. - The base `ApplicationsTable` received some basic refactoring to: - group like function together - utilize the `DecoratedApplication` Signed-off-by: Scott J Dickerson <[email protected]>
client/src/app/pages/applications/applications-table/components/column-application-name.tsx
Outdated
Show resolved
Hide resolved
client/src/app/pages/applications/applications-table/components/column-application-name.tsx
Outdated
Show resolved
Hide resolved
addon?: string; | ||
kind?: string; | ||
} | ||
export const TaskStates = { |
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.
follow-up candidate:
- enforce types at least on the "value" part (
TaskState[]
) - the keys are named almost like task state but there are differences that may cause bugs
client/src/app/pages/applications/applications-table/useDecoratedApplications.ts
Show resolved
Hide resolved
client/src/app/pages/applications/applications-table/useDecoratedApplications.ts
Outdated
Show resolved
Hide resolved
client/src/app/pages/applications/applications-table/useDecoratedApplications.ts
Outdated
Show resolved
Hide resolved
@sjd78
|
The tasks status looks better being calculated in the hook than in the component. Moved it there and readjusted the component. Now the summarized tasks status is available everywhere a `DecoratedApplication` is used. Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
Done!
Not much to do about that since the popover wraps
👍
I'm not worried about that one right now but good to think about in the near future. |
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.
LGTM
@sjd78 this looks great! |
Making executive decision to merge in favor of cutting the new alpha, will be fixing the tests tomorrow, thanks @sjd78 ! |
Summary
On the application table's name column, add a status icon. The icon summarizes the status of the various kinds of tasks attached to an application (a single application has 0 or more tasks while a task acts on a single application).
The icon will be driven off looking at the most recent task (by
createTime
) perkind
for an application:A popover on hover will render on the application name column to display summary information about the application's
tasks. The popover will have a table that shows the most recent tasks for each kind of task associated to the application.
Based on refactoring PR #1988 and PR #1989
Closes: #1934
Fixes: #1772
Change Details
Created
useDecoratedApplications()
hook to run all calculations and cross-referencing necessary to render most of the table.Created
ColumnApplicationName
to handle the application name column with the status icon and popover.Refactored
useFetchTasks()
hook to only do a simple sort. The filtering based on kind is no longer necessary.The base
ApplicationsTable
received some basic refactoring to utilize the content ofDecoratedApplication
Screen Shots
Base view with various application task statuses:
Popover details of an application with canceled tasks:
Popover details of an application with successful tasks:
Popover details of an application with failed tasks: