Skip to content

Conversation

@saxenanihal95
Copy link
Contributor

@saxenanihal95 saxenanihal95 commented Aug 15, 2020

Fixes: #2505

  • Removed Button component as it is tightly coupled
    to BottomSheet Component

  • Exposed isVisible prop so that user can easily
    manage BottomSheet Component visibililty

@codecov
Copy link

codecov bot commented Aug 15, 2020

Codecov Report

Merging #2507 into next will decrease coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #2507      +/-   ##
==========================================
- Coverage   93.10%   93.03%   -0.08%     
==========================================
  Files          40       40              
  Lines         769      761       -8     
  Branches      115      113       -2     
==========================================
- Hits          716      708       -8     
  Misses         49       49              
  Partials        4        4              
Impacted Files Coverage Δ
src/bottomSheet/BottomSheet.js 100.00% <100.00%> (ø)

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 3eb3377...6e5470d. Read the comment docs.

@saxenanihal95 saxenanihal95 changed the title 2505: Fix: Functionality to close the BottomSheet Fix: Functionality to close the BottomSheet Aug 15, 2020
@JoeDuncko
Copy link
Contributor

This looks like a much needed change! Question: is the onClose prop being used at all here? Should it be?

@saxenanihal95
Copy link
Contributor Author

@JoeDuncko Yes i was first thinking to provide onClose but that is already part of modalProps (onDismiss) but that is only supported for ios right now.

* Removed Button component as it is tightly coupled
to BottomSheet Component

* Exposed isVisible prop so that user can easily
manage BottomSheet Component visibililty
@JoeDuncko
Copy link
Contributor

Cool, that makes more sense to me now. I think it's a good first step to a much more useful BottomSheet component. Thanks so much for getting this in so quickly!

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.

Add a functionality that will close the BottomSheet when a list item is pressed

3 participants