-
Notifications
You must be signed in to change notification settings - Fork 4.5k
🪟 🔧 Generify GroupControls to make it more reusable #20004
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
Conversation
@@ -6,7 +6,6 @@ $group-spacing: variables.$spacing-xl; | |||
$border-width: variables.$border-thick; | |||
|
|||
.container { | |||
margin-bottom: 27px; |
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.
Removed this because we don't want this spacing in the Connector Builder. To not affect the look of the ConnectorForm, I opted to wrap the ArraySection and ConditionSection components in SectionContainers
instead: https://github.com/airbytehq/airbyte/pull/20004/files#diff-359ca8d25eceab9f588e4ca17095865f532588fecd0d7d9f2fc76458c1b1f10bR66
This is similar to how we add the correct spacing in FormSection
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.
Tested and seems to work fine, no regressions that I noticed. LGTM
|
||
.dropdown { | ||
margin-left: auto; | ||
padding: 0 2px; |
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.
nit: should we use a size variable here?
What
The Connector Builder needs to include inputs similar to the ConditionSection of the ConnectorForm, in which there is a label with a dropdown to select which fields are shown inside of the box. However, the styling and positioning of the dropdown is currently contained in the
ConditionSection
component, which has other assumptions and logic specific to the ConnectorForm.How
Instead, we would like to reuse the generic GroupControls component to implement this in the connector builder, so to avoid repeating the dropdown styling, this PR makes the label and dropdown explicit props, rather than taking in a single
title
prop that contains both, and moves the styling of these into the GroupControls scss file.This way, the Connector Builder can reuse the GroupControls component to get the same look as that in the ConnectorForm, without needing to repeat styling, such as in this feature branch.
🚨 User Impact 🚨
There should be no user-visible changes as a result of this PR.