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

feat(ui): enhance empty state handling for executions and selection in components #5557

Merged
merged 14 commits into from
Oct 22, 2024

Conversation

sikehish
Copy link
Contributor

What changes are being made and why?

This PR introduces two main features to improve user experience when interacting with data tables and charts in the application:

  1. Empty State Handling: Added an <el-empty> component to both the Bar and SelectTable components. This component is displayed when there are zero executions or no data available, enhancing the user interface by providing clear feedback to users.

  2. Conditional Rendering: Updated the rendering logic for the bulk select header in the SelectTable component to only show when there is data selected, ensuring a cleaner interface and preventing confusion when no data is available.

These changes enhance usability by clearly communicating to users when there is no data to display, thereby improving the overall experience.


How the changes have been QAed?

To QA the changes, the following flow demonstrates the user experience when there are zero executions or no data:

  1. Navigate to Executions, or Flows.
  2. Ensure that there are no executions(or flows).
  3. Observe that the <el-empty> component is correctly displayed in both the Bar and SelectTable components.
- Navigate to Executions, or Flows.
- If any executions/flows are present, you may delete them.
- Confirm that the empty state message is displayed correctly.

…ection header

- Added an <el-empty> component to display when data is not available
- Updated bulk select header to render only if there is selection and data
- Added <el-empty> component to display when total executions are 0
- Updated conditional rendering for info container and chart display based on total executions
@MilosPaunovic MilosPaunovic self-requested a review October 20, 2024 17:57
@MilosPaunovic MilosPaunovic self-assigned this Oct 20, 2024
Copy link
Member

@MilosPaunovic MilosPaunovic left a comment

Choose a reason for hiding this comment

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

This is great work so far @sikehish.

I have one small cleanup comment on this so far, but i'd like to ask you to add the same logic you've added to executions/Bar.vue component to the executions/Doughnut.vue and to logs/Bar.vue, to cover all of the new charts.

Also, in kestra/ui/src/components/executions/Executions.vue I would prefer to add <template #table v-if="executions.length"> this to line 143, and in kestra/ui/src/components/flows/Flows.vue something similar, <template #table v-if="flows.length"> to line 89, as it would look nicer if we don't ave two empty states on the same page, one below the other.

ui/src/components/layout/SelectTable.vue Outdated Show resolved Hide resolved
@Skraye
Copy link
Member

Skraye commented Oct 21, 2024

Also, can you share a screen of what it looks like ?
Is this displayed?
image

If yes, it needs to use the translation

@sikehish
Copy link
Contributor Author

This is great work so far @sikehish.

I have one small cleanup comment on this so far, but i'd like to ask you to add the same logic you've added to executions/Bar.vue component to the executions/Doughnut.vue and to logs/Bar.vue, to cover all of the new charts.

Also, in kestra/ui/src/components/executions/Executions.vue I would prefer to add <template #table v-if="executions.length"> this to line 143, and in kestra/ui/src/components/flows/Flows.vue something similar, <template #table v-if="flows.length"> to line 89, as it would look nicer if we don't ave two empty states on the same page, one below the other.

Thank you, will get this done ASAP :)

@sikehish
Copy link
Contributor Author

Also, can you share a screen of what it looks like ? Is this displayed? image

If yes, it needs to use the translation

Could you guide me with the translation part? I'll share the image in a while.

@Skraye
Copy link
Member

Skraye commented Oct 21, 2024

Sure, its something straightforward, first here is how you use it, with the $t() methods :

{{ $t('behavior') }}: <status class="mx-2" :status="flow.concurrency.behavior" />

And all you have to do, is to put the english translation here :

"files": "Files"

Our CI will handle translation in every other language 👍

- Added conditional rendering for the executions table in Executions.vue to avoid displaying two empty states on the same page.
- Implemented similar changes for the flows table in Flows.vue.
…n logic

- Added a new `total` prop to the Doughnut.vue component with type Number.
- Implemented similar logic from executions/Bar.vue into executions/Doughnut.vue for consistency in handling total executions.
- Added an `<el-empty>` component to display when `hasStatsData` is false.
- Enhanced validation logic in `<template #table>` on line 58 for improved data handling.
- Added translation for "no data" corresponding to "No data available" in `kestra/ui/src/translations/en.json`.
@sikehish
Copy link
Contributor Author

Sure, its something straightforward, first here is how you use it, with the $t() methods :

{{ $t('behavior') }}: <status class="mx-2" :status="flow.concurrency.behavior" />

And all you have to do, is to put the english translation here :

"files": "Files"

Our CI will handle translation in every other language 👍

Hi @Skraye. I've added the translation. Let me know if anything else needs to be added/modified.

- Wrapped the existing Bar component in a div that checks if `props.data.length > 0`.
- Added an <el-empty> component.
@sikehish
Copy link
Contributor Author

This is great work so far @sikehish.

I have one small cleanup comment on this so far, but i'd like to ask you to add the same logic you've added to executions/Bar.vue component to the executions/Doughnut.vue and to logs/Bar.vue, to cover all of the new charts.

Also, in kestra/ui/src/components/executions/Executions.vue I would prefer to add <template #table v-if="executions.length"> this to line 143, and in kestra/ui/src/components/flows/Flows.vue something similar, <template #table v-if="flows.length"> to line 89, as it would look nicer if we don't ave two empty states on the same page, one below the other.

Hi @MilosPaunovic
Ive implemented all of this.
However, I've made some changes in LogsWrapper.vue , as the logic and validation was already in place at the LogsWrapper level. I've also made changes in the logs/Bar.vue component( basically added an additional check, as it's being rendered in Dashboard). I realised that Bar.vue was being conditionally rendered in LogsWrapper so I modified the else condition in LogsWrapper.

@MilosPaunovic MilosPaunovic self-requested a review October 22, 2024 06:37
@MilosPaunovic MilosPaunovic merged commit f45e934 into kestra-io:develop Oct 22, 2024
3 of 4 checks passed
@MilosPaunovic
Copy link
Member

Just a note here, you've done a great job, I've just pushed small tweaks to have everything properly aligned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants