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.

Sign up for GitHub

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

Jump to bottom

Update form field labels in connection form to match design #21036

Merged
merged 10 commits into from
Jan 11, 2023
Merged

Update form field labels in connection form to match design #21036

merged 10 commits into from
Jan 11, 2023

Conversation

matter-q
Copy link
Contributor

@matter-q matter-q commented Jan 4, 2023 •
edited
Loading

What

Closes #19900

How

Replaced ControlLabels message property with infoTooltipContent

For test purposes, use the environment variable:

  • REACT_APP_NEW_STREAMS_TABLE=true
  • REACT_APP_NEW_STREAMS_TABLE=false

Figma design

Loom

https://www.loom.com/share/ea52bb672849427e91a2d72b603f40ce

https://www.loom.com/share/9e08684e93dc48f5b4944bc7cbb12fea

image

image

image

image

@matter-q matter-q requested a review from a team as a code owner January 4, 2023 20:43
@matter-q matter-q requested a review from YatsukBogdan1 January 4, 2023 20:43
@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Jan 4, 2023
YatsukBogdan1
YatsukBogdan1 approved these changes Jan 5, 2023
View reviewed changes
Copy link
Contributor

@YatsukBogdan1 YatsukBogdan1 left a comment

Choose a reason for hiding this comment

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

looks good to me

@matter-q matter-q requested a review from edmundito January 5, 2023 14:42
@edmundito
Copy link
Contributor

@matter-q from looking at the changes, the namespace/stream name prefix fields also need to be updated so that they are consistent with the current table.

@matter-q
Copy link
Contributor Author

matter-q commented Jan 5, 2023

@matter-q from looking at the changes, the namespace/stream name prefix fields also need to be updated so that they are consistent with the current table.

I didn't quite understand what needed to be updated...
Could you be more specific?

@edmundito
Copy link
Contributor

edmundito commented Jan 5, 2023 •
edited
Loading

When the new streams table feature is disabled (REACT_APP_NEW_STREAMS_TABLE=false), there are more fields.

Also, while creating a new connection, there are extra name and data residency fields. Here's what it looks like from the new connection page:

image

Also, the vertical alignment of the items should be centered but it is not:
image

edmundito
edmundito reviewed Jan 9, 2023
View reviewed changes
Copy link
Contributor

@edmundito edmundito left a comment

Choose a reason for hiding this comment

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

The changes look good, and tested locally. There is one more thing in terms of how the fields are presented in the new design. Required fields no longer need the * next to it and instead the optional fields are marked as optional (there is a prop in the ControlLabels to do this). Here's an example with the Destination Stream Prefix:

Screen Shot 2023-01-09 at 10 36 10

Mark Berger added 5 commits January 10, 2023 18:26
Update form field labels in connection form to match design
901a90d 
- Replaced ControlLabels message property with infoTooltipContent
Update form field labels in connection form to match design
71081f9 
- Replaced ControlLabels message property with infoTooltipContent
Update form field labels in connection form to match design
410cafe 
- Replaced ControlLabels message property with infoTooltipContent
- Replaced property "flex-start" to "center" for FlexContainer (New Connection)
Update form field labels in connection form to match design
89857e7 
- Replaced ControlLabels message property with infoTooltipContent
- Replaced property "flex-start" to "center" for FlexContainer (New Connection)
Update form field labels in connection form to match design
f8166d8 
- Replaced ControlLabels message property with infoTooltipContent
- Replaced property "flex-start" to "center" for FlexContainer (New Connection)
@matter-q matter-q force-pushed the mark/update-form-field-labels-in-connection-form-to-match-design branch from 06ad99f to f8166d8 Compare January 10, 2023 16:26
@octavia-squidington-iv octavia-squidington-iv removed the area/platform issues related to the platform label Jan 10, 2023
Update form field labels in connection form to match design
b2da157 
- Replaced ControlLabels message property with infoTooltipContent
- Replaced property "flex-start" to "center" for FlexContainer (New Connection)
@matter-q matter-q requested a review from edmundito January 10, 2023 16:51
Mark Berger added 2 commits January 10, 2023 18:54
Update form field labels in connection form to match design
4e30293 
- Replaced ControlLabels message property with infoTooltipContent
- Replaced property "flex-start" to "center" for FlexContainer (New Connection)
Update form field labels in connection form to match design
f4f65df 
- Replaced ControlLabels message property with infoTooltipContent
- Replaced property "flex-start" to "center" for FlexContainer (New Connection)
edmundito
edmundito suggested changes Jan 10, 2023
View reviewed changes
Copy link
Contributor

@edmundito edmundito left a comment

Choose a reason for hiding this comment

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

Tested locally, but noticed one small thing...

airbyte-webapp/src/views/Connection/ConnectionForm/ConnectionFormFields.tsx Outdated Show resolved Hide resolved
@edmundito
Update airbyte-webapp/src/views/Connection/ConnectionForm/ConnectionF…
5c552da 
…ormFields.tsx

Co-authored-by: Edmundo Ruiz Ghanem <168664+edmundito@users.noreply.github.com>
@matter-q matter-q requested a review from edmundito January 10, 2023 21:11
@edmundito
Copy link
Contributor

edmundito commented Jan 10, 2023 •
edited
Loading

Sorry, just tested the cloud version and noticed a couple of things on the connection settings page:

image

  1. The tooltip should now use the "learn more" link for tooltips: https://63053d1047904393cf8d8faa-hpmeogsbgz.chromatic.com/?path=/story/ui-tooltip--with-learn-more-url
  2. The spacing between the label and the dropdown needs to be adjusted. The dropdown now takes whatever is left of the card, but it should be a specific width (300px) and looks like removing the description broke the layout

Update form field labels in connection form to match design
1358c0a 
- Replaced ControlLabels message property with infoTooltipContent
@matter-q matter-q merged commit 4b5cf8b into master Jan 11, 2023
@matter-q matter-q deleted the mark/update-form-field-labels-in-connection-form-to-match-design branch January 11, 2023 14:01
@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Jan 12, 2023
Merged
jbfbell pushed a commit that referenced this pull request Jan 13, 2023
@jbfbell
Update form field labels in connection form to match design (#21036)
be86bd5 
* Update form field labels in connection form to match design

- Replaced ControlLabels message property with infoTooltipContent
- Replaced property "flex-start" to "center" for FlexContainer (New Connection)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YatsukBogdan1 YatsukBogdan1 YatsukBogdan1 approved these changes

@edmundito edmundito edmundito approved these changes

Assignees
No one assigned
Labels
area/frontend Related to the Airbyte webapp
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update form field labels in connection form to match design
4 participants
@matter-q @edmundito @YatsukBogdan1 @octavia-squidington-iv