-
Notifications
You must be signed in to change notification settings - Fork 33
refactor(ui): Created common Empty State Component #228
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
base: staging
Are you sure you want to change the base?
refactor(ui): Created common Empty State Component #228
Conversation
* chore: add comments * chore: 🚨 lint fix
…atazip-inc#195) * feat: add error logs i check conenction for source and destination * fix: parse json message as string * chore: update response key * chore: refactor * chore: remove logEntry struct * fix: log parsing * fix: log parsing * fix: logs parsing logic * docs: updated api contract
…ogs-in-test-connection feat: show error logs in test connection
…omponent with configs
|
@tarakaswathi-datazip please review this PR, and let me know if any changes are necessary. |
|
hey thanks a lot for your contribution @aarya-16 can you also go ahead and share the pr on our slack in contribute-to-olake channel? |
* test: ✅ base setup for automation testing using playwright , add test cases fro major flows * fix: integration workflow changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * fix: ci changes * test: add postgres flow * fix: lint changes * test: add iceberg specific test * fix: tests * fix: stream name * fix: test fix * fix: test fix * fix: test fix * fix: test fix * fix: changes * fix: ci changes * fix: ci changes * fix: ci changes * test: fail test on test connection failure * fix: ci changes * fix: changes * fix: changes * fix: changes * fix: changes * fix: changes * fix: changes * fix: test fix * fix: changes * fix: changes * fix: test-data fix * fix: test-data fix * fix: changes * fix: changes * fix: changes * fix: host and port changes * fix: changes * fix: change * test: login test updated * test: refactor create source and destination page * test: refactor tests * fix: test fix * fix: changes * fix: fix changes * fix: changes * fix: changes * fix: review changes * refactor: refactor tests to use authenticated state of playwright * fix: changes * docs: update tests readme * test: intercept and log destination spec response * docs: update readme * fix: update timeouts for playwright tests * test: update readme * fix: add retries in playwright config * test: fix timeout duration * fix: update readme * test: add connector data test-id * fix: resolve comments * refactor(test): make and use enums instead of strings * fix: fix test data builder * fix: test data job name * fix: add todo --------- Co-authored-by: Duke Dhal <[email protected]> Co-authored-by: deepanshupal09-datazip <[email protected]>
|
Hi @aarya-16, have you tested your changes? can you share screenshot of your changes made? |
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 goal of this issue was to consolidate all Empty State components into a single shared one
Please remove this component and use the common EmptyState.tsx instead
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.
same for this
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.
and this too
ui/src/types/commonTypes.ts
Outdated
| export interface EmptyStateConfig { | ||
| image: string | ||
| welcomeText: string | ||
| welcomeTextColor: string | ||
| heading: string | ||
| description: string | ||
| descriptionColor: string | ||
|
|
||
| button: { | ||
| text: string | ||
| icon: string | ||
| className: string | ||
| } | ||
|
|
||
| tutorial: { | ||
| link: string | ||
| image: string | ||
| altText: string | ||
| } | ||
| } | ||
|
|
||
| export interface EmptyStateProps { | ||
| config: EmptyStateConfig | ||
| onButtonClick: () => void | ||
| } |
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 don’t need to pass this much information into the EmptyState component. Instead, the props can be simplified to something like:
export interface EmptyStateProps {
page: "job" | "sources" | "destinations";
onButtonClick: () => void
}
The config logic can live inside the EmptyState component itself... since it’s only used there and not reused elsewhere. This way, we can conditionally render based on the page prop instead of passing the entire config through props.
ui/src/utils/emptyStateConfigs.ts
Outdated
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.
As mentioned in the above comment, please remove this file from utils, and keep the config inside the EmptyState Component itself
| @@ -0,0 +1,87 @@ | |||
| import { Button } from "antd" | |||
| import { PlayCircle, Plus, GitCommit } from "@phosphor-icons/react" | |||
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.
Please change this to
| import { PlayCircle, Plus, GitCommit } from "@phosphor-icons/react" | |
| import { PlayCircleIcon, PlusIcon, GitCommitIcon } from "@phosphor-icons/react" |
as these are depracted icon, and there is a separate PR to replace them everywhere, so let's keep it consistent here too
ui/src/utils/emptyStateConfigs.ts
Outdated
| image: "/src/assets/FirstDestination.svg", | ||
| welcomeText: "Welcome User !", | ||
| welcomeTextColor: "text-brand-blue", | ||
| heading: "Ready to create your first destination", | ||
| description: | ||
| "Get started and experience the speed of OLake by running jobs", | ||
| descriptionColor: "text-text-primary", | ||
| button: { | ||
| text: "New Destination", | ||
| icon: "Plus", | ||
| className: | ||
| "border-1 mb-12 border-[1px] border-[#D9D9D9] bg-white px-6 py-4 text-black", | ||
| }, | ||
| tutorial: { | ||
| link: "https://youtu.be/Ub1pcLg0WsM?si=V2tEtXvx54wDoa8Y", | ||
| image: "/src/assets/DestinationTutorial.svg", | ||
| altText: "Destination Tutorial", | ||
| }, |
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.
Also, I don't think we need these many things in the config... some attributes here are common in all three configs(job, source, destination)
Please remove such attributes that are common, and try to keep the attributes here to minimum, things that are actually different in all three.
ui/src/types/commonTypes.ts
Outdated
| export interface EmptyStateConfig { | ||
| image: string | ||
| welcomeText: string | ||
| welcomeTextColor: string | ||
| heading: string | ||
| description: string | ||
| descriptionColor: string | ||
|
|
||
| button: { | ||
| text: string | ||
| icon: string | ||
| className: string | ||
| } | ||
|
|
||
| tutorial: { | ||
| link: string | ||
| image: string | ||
| altText: string | ||
| } | ||
| } | ||
|
|
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.
Since we are moving the config to the component itself, also remove this interface from here
* Added success message on Job History refetch * Update ui/src/modules/jobs/pages/JobHistory.tsx Co-authored-by: deepanshupal09-datazip <[email protected]> --------- Co-authored-by: vikash choudhary <[email protected]> Co-authored-by: deepanshupal09-datazip <[email protected]>
…datazip-inc#216) * fix: replace deprecated Phosphor icons with latest *Icon alternatives * fix: update Phosphor icons to latest alternatives in DestinationEdit and SourceEdit components and fix Lint issue * refactor: remove EditSourceModal * fix: update icon import --------- Co-authored-by: vikash choudhary <[email protected]>
* feat: add append only mode in ui * fix: data filter bug * fix: fix color of append only mode switch * fix: add custom option in all streams ingestion mode change * fix: use clsx for conditional styling * fix: fix icons
|
@deepanshupal09-datazip Here are the screenshots: |
0c10a25 to
8335e7e
Compare



Description
Overview
Replaced three duplicate empty state components (
DestinationEmptyState,SourceEmptyState,JobEmptyState) with a single reusableEmptyStatecomponent, while preserving existing functionality and visual styling.File Changes
New Files Added
ui/src/modules/common/components/EmptyState.tsxui/src/utils/emptyStateConfigs.tsFiles Modified
ui/src/types/commonTypes.tsEmptyStateConfigandEmptyStatePropsinterfaces for TypeScript supportui/src/modules/destinations/components/DestinationEmptyState.tsxEmptyStatecomponent with destination configurationui/src/modules/sources/components/SourceEmptyState.tsxEmptyStatecomponent with source configurationui/src/modules/jobs/components/JobEmptyState.tsxEmptyStatecomponent with job configurationfixes #220
Type of change
How Has This Been Tested?
Related PR's (If Any):