srivastavaanurag79 / react-native-paper-select

Dropdown using React Native Paper TextInput, Checkbox and Dialog
https://anurag-srivastava.gitbook.io/react-native-paper-select/
MIT License
48 stars 15 forks source link

Switched the CheckboxInput component to use Checkbox.Item to stay mor… #56

Closed himrocks33 closed 2 months ago

himrocks33 commented 3 months ago

Switched the CheckboxInput component to use Checkbox.Item to stay more consistent with RN paper and allow for the mode parameter.

This will use Text from RN Paper for the label as well keeping the theme consistent.

himrocks33 commented 3 months ago

@Thodor12 Please note in this PR I have set the 'position' property to 'leading'. This puts the checkbox on the left side of the text. The implementation is the same as before. It just stays more consistent with React Native Paper's styling. It also adds the mode property. Thanks for the review.

himrocks33 commented 2 months ago

Now also includes bug fixes for #53 and #49 as per @srivastavaanurag79 request.

@CarlosSLoureiro or @Thodor12 could you please review?

Thodor12 commented 2 months ago

With all the individual property additions, isn't it easier to just inherit the typings from the paper checkbox and using rest parameters to pass everything at once to the checkbox? (Whilst making sure to omit certain things we have to explicitly set)

himrocks33 commented 2 months ago

@Thodor12 UI changes are minimal. Seems like you are grilling me for no reason on this PR. It’s pretty straightforward.

srivastavaanurag79 commented 2 months ago

@himrocks33 Please add this feature to this merge request,i.e, limiter, a key name limit which limits the maximum number of items one can select in the multi-select.

Future Features #59 :

  1. Sectional List i.e. see below example
  1. Chips style selected items (optional styling)
  2. Different View For Single Select, example below 1_rQNkcjLw1RYtDZRNqoyQtw
Thodor12 commented 2 months ago

@Thodor12 UI changes are minimal. Seems like you are grilling me for no reason on this PR. It’s pretty straightforward.

If you think that that is your problem.

All I'm asking for is a simple before and after. I see a bunch of changes and I've got no idea of the impact of this. If things start changing between versions we have to state it as a breaking change, otherwise people can get silently a new interface without knowing if they update the library.

srivastavaanurag79 commented 2 months ago

It will be a breaking change since the checkbox style is being changed and some other new features are being added as well. The pull request will be strictly reviewed so please be patient and please be a bit polite everyone.

srivastavaanurag79 commented 2 months ago

@Thodor12 If you have time please switch to @himrocks33 branch and run the project to review the changes and update it with your guidance if you find something unusual or not needed.

himrocks33 commented 2 months ago

I got pretty busy at work again for the time being. I feel like there is enough here for a feature release though. @srivastavaanurag79

himrocks33 commented 2 months ago

@srivastavaanurag79 #59 items seem like a complete rewrite almost to support the single select (which would probably be a whole new component really?) I doubt I have time for that right now. Bug Fixes and minor changes I can do. Where do we stand with getting this PR merged?

srivastavaanurag79 commented 2 months ago

@himrocks33 #59 you Don't have to do them, those are future features

srivastavaanurag79 commented 2 months ago

@himrocks33 if adding a limiter feature is possible please add it, a key name limit which limits the maximum number of items one can select in the multi-select, will try to review and merge this pull request this Sunday by myself if other contributors don't have time for the same.

srivastavaanurag79 commented 2 months ago

I dont know why but disable property isn't working as it is supposed to be, it still lets me check/uncheck. and using Checkbox.Item has changed the view dramatically.

Sharing the screenshot, longer text are not visible properly and as you can select the label now, but the select area is restricted to the size of the label.

Screenshot 2024-04-18 at 12 31 29 PM
srivastavaanurag79 commented 2 months ago

Fixed disable issue and checkbox label issue, select all issue

Screenshot 2024-04-18 at 2 53 21 PM