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

Code cleanup #33

Closed Thodor12 closed 1 year ago

Thodor12 commented 1 year ago

Revamped the entire documentation to better reflect all the properties, aswell as describe newly added properties. Code changes to remove unnecessary properties, merged certain properties into objects, used proper typings where possible.

Property renames:

Property type changes:

Other changes:

No functionality should work differently after this PR, short of property changes, so it will be a breaking change. Users may see differences in styling in some places (for example the defaulting to "flat", better respecting MUI theme, etc)

resolves #31

Thodor12 commented 1 year ago

First finish #32 so changes can be merged into here

srivastavaanurag79 commented 1 year ago

Please add the issue #31 feature needed i.e. the left icon prop in the text input. And thank you for your work.

Thodor12 commented 1 year ago

@srivastavaanurag79 added that option too, could you look at #32 so I can merge that back into here?

Thodor12 commented 1 year ago

Dialog label changes have been ported into here instead:

Currently it's not possible to separately style the close and done buttons. In my application I'm consistently holding to a "error" style for cancel buttons and a "primary" style for completion/save buttons. This is not doable if there's only 1 property available for setting the style for both buttons.

Suggested changes:

srivastavaanurag79 commented 1 year ago

Error: Unable to resolve module react-native-paper/lib/typescript/src/core/theming

srivastavaanurag79 commented 1 year ago

did you remove the icon and everything?

srivastavaanurag79 commented 1 year ago

can you please attach some screenshots of what it looks like now?

srivastavaanurag79 commented 1 year ago

why have you wrapped it again inside a provider? people who would add react-native-paper will add their main app inside provider.

Thodor12 commented 1 year ago

Error: Unable to resolve module react-native-paper/lib/typescript/src/core/theming

I pulled the repo and installed react-native-paper like normal, it could be there's a package update which prevents you from building it locally.

did you remove the icon and everything?

Some properties have been moved to textInputProps, checkboxProps and searchbarProps as they don't warrant having to make a first class additional property. No original functionality has been removed.

can you please attach some screenshots of what it looks like now?

Will do a bit later, is there a way I can actually run the app locally. Is it possible to run the example using local version of the code?

why have you wrapped it again inside a provider? people who would add react-native-paper will add their main app inside provider.

There's 2 different providers, PaperProvider and ThemeProvider. The first one is from react-native-paper themselves to supply the theme prop aswell as do other things underwater. ThemeProvider is the actual supply mechanism for the theme to MUI components. You can see that behaviour here: https://github.com/callstack/react-native-paper/blob/main/src/core/Provider.tsx and https://github.com/callstack/react-native-paper/blob/main/src/core/theming.tsx#L31

By doing it this way you don't have to manually pass theme={theme} to every single component as the react context will simply utilize the closest ThemeProvider.

srivastavaanurag79 commented 1 year ago

You changed a lot of things, and it isn't working in my system. It keeps throwing the error I sent you react-native-paper v5.5.1 is installed for me previously I was using v4.9.2

Thodor12 commented 1 year ago

You changed a lot of things, and it isn't working in my system. It keeps throwing the error I sent you react-native-paper v5.5.1 is installed for me previously I was using v4.9.2

My version is 5.5.1 too, could you maybe clean your node_modules? The import is correct, also:

Will do a bit later, is there a way I can actually run the app locally. Is it possible to run the example using local version of the code?

srivastavaanurag79 commented 1 year ago

You changed a lot of things, and it isn't working in my system. It keeps throwing the error I sent you react-native-paper v5.5.1 is installed for me previously I was using v4.9.2

My version is 5.5.1 too, could you maybe clean your node_modules? The import is correct, also:

Will do a bit later, is there a way I can actually run the app locally. Is it possible to run the example using local version of the code?

I am running the local version of the example and the error still persists. Done clean install still same error:

Error: Unable to resolve module react-native-paper/lib/typescript/src/core/theming

ThemeProvider can be imported directly from react-native-paper but the second import requires this path and its throwing error

Thodor12 commented 1 year ago

@srivastavaanurag79 I changed the imports and swapped useInternalTheme to useTheme. I cannot get the example to work on my local machine (it won't import the local changes to the package) so I am not able to actually check how it all works now