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.

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

Add AccountSelector to Box and Field children #2774

Merged
merged 4 commits into from
Sep 30, 2024

Conversation

GuillaumeRx
Copy link
Contributor

@GuillaumeRx GuillaumeRx commented Sep 30, 2024

This PR fixes the forgotten addition of the AccountSelector to Box childrens list.

It also allows the usage of AccountSelector in the Field component.

The title prop of the AccountSelector was also dropped in favor of wrapping this component inside of a Field.

@GuillaumeRx GuillaumeRx requested a review from a team as a code owner September 30, 2024 13:47
@Maliksb11

This comment was marked as spam.

@FrederikBolding
Copy link
Member

@GuillaumeRx Needs some updates to tests it seems

@GuillaumeRx
Copy link
Contributor Author

@GuillaumeRx Needs some updates to tests it seems

Yes, I'm also adding more things to this PR. Hold on :)

@GuillaumeRx GuillaumeRx changed the title Add AccountSelector to Box children Add AccountSelector to Box and Field children Sep 30, 2024
@@ -341,7 +341,6 @@ export const AccountSelectorStruct: Describe<AccountSelectorElement> = element(
'AccountSelector',
{
name: string(),
title: string(),
Copy link
Member

Choose a reason for hiding this comment

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

Does the account selector have a static title now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dropped this in favour of the usage of the Field's label prop :)

Copy link

codecov bot commented Sep 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.41%. Comparing base (e7fab1c) to head (e707ce8).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2774   +/-   ##
=======================================
  Coverage   94.41%   94.41%           
=======================================
  Files         482      482           
  Lines       10261    10261           
  Branches     1563     1563           
=======================================
  Hits         9688     9688           
  Misses        573      573           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GuillaumeRx GuillaumeRx merged commit 6cc4791 into main Sep 30, 2024
165 of 166 checks passed
@GuillaumeRx GuillaumeRx deleted the gr/add-account-selector-to-form branch September 30, 2024 14:56
GuillaumeRx added a commit that referenced this pull request Oct 4, 2024
GuillaumeRx added a commit that referenced this pull request Oct 4, 2024
GuillaumeRx added a commit that referenced this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants