Skip to content

🪟 🔧 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

Merged
merged 3 commits into from
Dec 6, 2022

Conversation

lmossman
Copy link
Contributor

@lmossman lmossman commented Dec 1, 2022

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.

@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Dec 1, 2022
@@ -6,7 +6,6 @@ $group-spacing: variables.$spacing-xl;
$border-width: variables.$border-thick;

.container {
margin-bottom: 27px;
Copy link
Contributor Author

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

@lmossman lmossman marked this pull request as ready for review December 1, 2022 22:44
@lmossman lmossman requested a review from a team as a code owner December 1, 2022 22:44
Copy link
Contributor

@flash1293 flash1293 left a 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;
Copy link
Contributor

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?

@lmossman lmossman merged commit 94477e4 into master Dec 6, 2022
@lmossman lmossman deleted the lmossman/generify-group-controls branch December 6, 2022 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants