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

refactor(react): Add fallback default function parammeter and change Condition's type #106

Conversation

Collection50
Copy link
Contributor

What I did

  1. The fallback parameter is optional, so we provided the default value.
  2. The condition function does not use parameters in the test code and business logic code, so I deleted the parameters.

Overview

Before

type Condition = boolean | ((...args: any[]) => boolean);

// ...

export const When = ({
  // ...
  fallback,
}: PropsWithChildren<WhenProps>) => {
 // ...
};

After

type Condition = boolean | (() => boolean);

// ...

export const When = ({
  // ...
  fallback = null,
}: PropsWithChildren<WhenProps>) => {
  // ...
};

PR Checklist

  • All tests pass.
  • All type checks pass.
  • I have read the Contributing Guide document.
    Contributing Guide

…Condition's type

Before:

type Condition = boolean | ((...args: any[]) => boolean);

After:

type Condition = boolean | (() => boolean);
@Collection50 Collection50 requested a review from ssi02014 as a code owner May 6, 2024 07:31
Copy link

changeset-bot bot commented May 6, 2024

🦋 Changeset detected

Latest commit: 8004942

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@modern-kit/react Patch
docs Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@ssi02014 ssi02014 left a comment

Choose a reason for hiding this comment

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

Thank you for creating a pull request 🤗

The args of the function type of the Condition seems to be unnecessary.

It also looks good that you assigned default values to the fallback props. Thanks! ❤️

Copy link

codecov bot commented May 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.88%. Comparing base (911f046) to head (8004942).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #106      +/-   ##
==========================================
+ Coverage   82.85%   82.88%   +0.03%     
==========================================
  Files          67       67              
  Lines         560      561       +1     
  Branches      118      119       +1     
==========================================
+ Hits          464      465       +1     
  Misses         84       84              
  Partials       12       12              
Components Coverage Δ
@modern-kit/react 72.59% <100.00%> (+0.08%) ⬆️
@modern-kit/utils 97.81% <ø> (ø)

@ssi02014 ssi02014 merged commit ef1094d into modern-agile-team:main May 6, 2024
3 checks passed
@github-actions github-actions bot mentioned this pull request May 5, 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.

2 participants