Skip to content

fix(CheckBox): Added onValueChange prop in checkbox - #39

Merged
artalat merged 5 commits into
developfrom
fix/checkboxchange
Jun 19, 2019
Merged

artalat merged 5 commits into
developfrom
fix/checkboxchange

Conversation

@danial24

Copy link
Copy Markdown

Handled onValueChange with onPress callback

onPress will behave as a callback without any parameters but onValueChange will have value and checked parameters.

@codecov

codecov Bot commented Jun 10, 2019 •

Copy link
Copy Markdown

Codecov Report

Merging #39 into develop will decrease coverage by 1.27%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #39      +/-   ##
===========================================
- Coverage      100%   98.72%   -1.28%     
===========================================
  Files           57       57              
  Lines          309      314       +5     
  Branches        30       32       +2     
===========================================
+ Hits           309      310       +1     
- Misses           0        4       +4
Impacted Files Coverage Δ
src/components/Checkbox/Checkbox.tsx 82.6% <33.33%> (-17.4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96776d4...bc74705. Read the comment docs.

@codecov-io

codecov-io commented Jun 10, 2019 •

Copy link
Copy Markdown

Codecov Report

Merging #39 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           develop    #39   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files           57     57           
  Lines          309    322   +13     
  Branches        30     35    +5     
======================================
+ Hits           309    322   +13
Impacted Files Coverage Δ
src/components/Checkbox/Checkbox.tsx 100% <100%> (ø) ⬆️
src/index.ts 100% <0%> (ø) ⬆️
src/withReactNativePaper.tsx 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96776d4...9a1d5f6. Read the comment docs.

Comment thread src/components/Checkbox/Checkbox.tsx Outdated
}

if (onValueChange) {
onValueChange(value, checked);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the new value sent forward correct? Or should it be !checked?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It will pass the current value of the checkbox before alteration.
This is the behaviour followed by material ui
React Native paper only have an onPress which does not provide you any parameters and it actually expects you to alter the current state manually.

@artalat
artalat merged commit 4cd1ec4 into develop Jun 19, 2019
@artalat
artalat deleted the fix/checkboxchange branch June 19, 2019 11:08
@artalat

artalat commented Jun 19, 2019

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants