🪟 🎨 Refining BulkEditPanel component #20474
Conversation
YatsukBogdan1
requested review from
dizel852 and
matter-q
and removed request for
a team
octavia-squidington-iv
added
area/platform
Would that be in cases where all of the selected streams do not have at least one column with the same name? Are there any other cases? |
|
Failing snapshot tests :) |
| ); | ||
| node = props.isMulti ? props.path.map(pathDisplayName).join(", ") : pathDisplayName(props.path); | ||
| } else { | ||
| node = <FormattedMessage id="connection.catalogTree.sourceDefined" />; |
There was a problem hiding this comment.
Nit: This could be in a useMemo
@tealjulia for now we decided to put only design updates in this PR, and keep any logic updates in separate one
@krishnaglick as we discussed on the stand up yesterday, the component itself is dumb, and just renders what is passed from |
There was a problem hiding this comment.
I re-tested again and did notice a discrepancy in the table itself. It looks like the non-selectable values have a background, but that only applies to the bulk edit
Here you can see that id is not selectable but it has the pill background around it
But this is really similar to what's needed in #20731
|
@edmundito I updated table row cursor and primary key as you suggested
|
…ty pillIfChangeUnavailable for cases when we need or not to render pill under unavailable to change path text
|
Added a few more updates:
|
|
@YatsukBogdan1 Just checked the latest changes and I noticed that what I see doesn't match the screenshot int he previous comment. |
|
updated PR with next changes:
|
airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx
Outdated
Show resolved
Hide resolved
| @@ -34,7 +36,7 @@ export const PillSelect: React.FC<PillSelectProps> = ({ className, ...props }) = | |||
| active={isOpen} | |||
| className={className} | |||
| > | |||
| {label} | |||
| {disabled ? <FormattedMessage id="connectionForm.bulkEdit.pillButtonLabel.notAvailable" /> : label} | |||
There was a problem hiding this comment.
This formatted message is part of the connection form, but this component is part of the UI, so it crosses different domains. We're not sure if the default behavior for disabled is always to say "not available"
There was a problem hiding this comment.
@edmundito I added renderDisabledState: () => React.ReactNode property for PillSelect, so now, when disabled: true, the content can be controlled from different application parts. please let me know if this looks good to you
| <PillButton disabled variant={variant} className={styles.streamPathSelect}> | ||
| {SourceDefinedNode} | ||
| </PillButton> |
There was a problem hiding this comment.
Following up on my comment on PillSelect, this code could be used to render when selecting the path is not available, instead of having to modify the PillButton.
# Conflicts: # airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx # airbyte-webapp/src/components/ui/PillSelect/PillSelect.tsx
* Adds handling for source defined and unavailable stream paths inside PillSelect * Adds handling of unavailable cursor and primary key field for PillButton components * Adds useMemo to SyncPathSelect component * Add InfoText component for cursor field and primary key to match table design in Figma * Removes animation for BulkEditPanel; Adds StreamPathSelect new property pillIfChangeUnavailable for cases when we need or not to render pill under unavailable to change path text * Disables pointer event while PillButton is disabled * Adds renderDisabledState property to control PillSelect content while disabled * Changes renderDisabledState -> disabledLabel
What
Closes #20372
Closes #20731
How
Switchstyling will be done in separate PRScreenshots