feat(ui/ingestion): add empty and loading states for sources and secrets tables #13646
Conversation
datahub-cyborg
bot
added
the
needs-review
|
🔴 Meticulous spotted visual differences in 3 of 1619 screens tested: view and approve differences detected. Meticulous evaluated ~10 hours of user flows against your PR. Last updated for commit 8183245. This comment will update as new commits are pushed. |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. ❌ Your patch check has failed because the patch coverage (30.76%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage. 📢 Thoughts on this report? Let us know! |
| @@ -1,19 +1,19 @@ | |||
| import { Icon, Pagination, SearchBar, Table, colors } from '@components'; | |||
| import { Empty, Modal, Typography, message } from 'antd'; | |||
| import { Modal, Typography, message } from 'antd'; | |||
There was a problem hiding this comment.
We could remove this antd modal as well. I think ConfirmationModal would work for this use-case
datahub-cyborg
bot
added
pending-submitter-response
| const EmptySources = ({ sourceType, isEmptySearch }: Props) => { | ||
| return ( | ||
| <EmptyContainer> | ||
| {isEmptySearch ? ( |
There was a problem hiding this comment.
Should this be other way round?
It still works here because we are doing !!query but it should be !query to be aligned with the variable name.
| {tableData.length === 0 ? ( | ||
| <EmptyState /> | ||
| {!loading && totalSecrets === 0 ? ( | ||
| <EmptySources sourceType="secrets" isEmptySearch={!!query} /> |
There was a problem hiding this comment.
yeah agreed with saketh that isEmptySearch and !!query don't match - isEmptySearch should mean there's no query? regardless i think we should make this a little clearer
Linear tickets:
https://linear.app/acryl-data/issue/CH-372/add-empty-and-loading-states-for-ingestion-table
https://linear.app/acryl-data/issue/CH-378/change-copy-in-execution-run-modal
Description:
Screenshots: