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

[Checkbox Group, Checkbox]: Adjust Checkbox Group's FormGroup and Checkbox's FormControl type #501

Merged
merged 11 commits into from
Dec 5, 2024

Conversation

videoeero
Copy link
Contributor

@videoeero videoeero commented Dec 3, 2024

Jira ticket: https://funidata.atlassian.net/browse/DS-410

Application developers encountered Typescript typing error with Checkbox Group. After discussion this PR's adjustments were made to address the issue.

Breaking changes:

  • Previously FormControl of Checkbox could be type of boolean | null | undefined. After this PR it can be boolean | null without undefined.
  • CheckboxGroup component now requires type parameter of FormGroup's type
    • E.g. before: element: CheckboxGroup --> element: CheckboxGroup<FudisCheckboxGroupFormGroup> or element: CheckboxGroup<MyType>

Slack: https://sisudev.slack.com/archives/C03G46K6LG1/p1733216094386739

@videoeero videoeero added improvement Improve or extend something that already exists bug fix Something isn't working as expected and removed improvement Improve or extend something that already exists labels Dec 3, 2024
@videoeero videoeero added breaking change This change is not backwards-compatible and removed bug fix Something isn't working as expected labels Dec 4, 2024
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 4, 2024 09:04 Destroyed
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 4, 2024 09:54 Destroyed
@videoeero videoeero changed the title [Checkbox Group]: Adjust FormGroup type [Checkbox Group, Checkbox]: Adjust Checkbox Group's FormGroup and Checkbox's FormControl type Dec 4, 2024
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 4, 2024 10:03 Destroyed
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 4, 2024 10:40 Destroyed
RiinaKuu
RiinaKuu previously approved these changes Dec 4, 2024
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 4, 2024 16:06 Destroyed
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 4, 2024 17:31 Destroyed
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 5, 2024 08:20 Destroyed
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 5, 2024 11:16 Destroyed
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 5, 2024 12:05 Destroyed
@github-actions github-actions bot temporarily deployed to Documentation for branch checkbox-group-formgroup-type-adjust December 5, 2024 12:49 Destroyed
Copy link
Contributor

@RiinaKuu RiinaKuu left a comment

Choose a reason for hiding this comment

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

Nice work, also discussed and decided that if/when other similar typing issues occur, fix them at that time, but for now this PR consist adjustments only for CheckboxGroup.

@RiinaKuu RiinaKuu merged commit 206f03b into main Dec 5, 2024
10 checks passed
@RiinaKuu RiinaKuu deleted the checkbox-group-formgroup-type-adjust branch December 5, 2024 14:30
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change This change is not backwards-compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants