-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
🪟 🎨 Row highlighting for new streams table #19449
Conversation
@@ -46,38 +49,74 @@ export const CatalogTreeTableRow: React.FC<StreamHeaderProps> = ({ | |||
[syncMode, destinationSyncMode] | |||
); | |||
|
|||
const { initialValues } = useConnectionFormService(); |
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.
No more prop drilling to find out if the row changed! We can do this at the row level now that we have our context.
This will also make it relatively straightforward to grab the diff from the context and apply it to some of the conditional logic below when we get to the point where we want to integrate the diff on here 💥
197fe09
to
1dd9e0f
Compare
airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useRowStatus.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss
Show resolved
Hide resolved
b559c35
to
724c8c8
Compare
@@ -52,6 +52,8 @@ $orange-800: #d83c24; | |||
$orange-900: #bf2f1b; | |||
$orange: $orange-400; | |||
|
|||
$green-30: #f4fcfd; |
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.
added these as labeled in Figma
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx
Outdated
Show resolved
Hide resolved
airbyte-webapp/src/components/connection/CatalogTree/next/useCatalogTreeTableRowProps.test.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Krishna (kc) Glick <krishnaglick@users.noreply.github.com>
…atalogTreeTableRowProps.test.tsx Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com>
…atalogTreeTableRowProps.tsx Co-authored-by: Krishna (kc) Glick <krishna@airbyte.io>
…atalogTreeTableRowProps.tsx Co-authored-by: Krishna (kc) Glick <krishna@airbyte.io>
…atalogTreeTableRowProps.tsx Co-authored-by: Krishna (kc) Glick <krishna@airbyte.io>
…atalogTreeTableRowProps.test.tsx Co-authored-by: Krishna (kc) Glick <krishna@airbyte.io>
…atalogTreeTableRowProps.test.tsx Co-authored-by: Krishna (kc) Glick <krishna@airbyte.io>
7bfed5d
to
e9cb95f
Compare
@@ -4,6 +4,10 @@ | |||
white-space: nowrap; | |||
} | |||
|
|||
.pillSelect { | |||
.streamPathSelect { |
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.
not an essential piece of this PR, but while digging around in the rows, I saw that the streamPathSelect did not stay the full width of its column like on Figma. Adjusted that.
options={options} | ||
value={props.path} | ||
isMulti={props.isMulti} | ||
onChange={(options: PathPopoutProps["isMulti"] extends true ? Array<{ value: Path }> : { value: Path }) => { | ||
const finalValues = Array.isArray(options) ? options.map((op) => op.value) : options.value; | ||
props.onPathChange(finalValues); | ||
}} | ||
className={styles.streamPathSelect} |
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.
again, adjusting this to be full column-width. not related to the main purpose of this PR, but a small, quick fix.
What
closes #19092
Addresses row highlighting functionality for new sync catalog table
How
Creates a hook
useCatalogTreeTableRowProps
that generates the custom styling forCatalogTreeTableRow
including:Also adds testing for the hook. Handles cases where a user may satisfy one or more state (ie: how to handle a row that has been both enabled and changed, how to handle a row that has been both enabled and selected for bulk mode, etc.)
Note: Disabled rows can still be edited and selected for BulkEdit. This should likely be addressed, but was not in scope of this PR.
Recommended reading order
useCatalogTreeTableRowProps
files... starting with the tests will help outline expected functionalityCatalogTreeTableRow
_colors.scss
,SyncModeSelect
,StreamPathSelect
,ModificationIcon
)