Skip to content

Feature/picker component#56

Open
adnan1naeem wants to merge 24 commits into
developfrom
feature/pickerComponent
Open

Feature/picker component#56
adnan1naeem wants to merge 24 commits into
developfrom
feature/pickerComponent

Conversation

@adnan1naeem

@adnan1naeem adnan1naeem commented Aug 6, 2019

Copy link
Copy Markdown
Contributor

Picker component with different modes modal,actionSheet and Menu for android and ios.
1:picker with mode modal popup opens and you can select items through listItem in android and ios
2:picker mode actionsheet : the picker opens in actionsheet in ios and android
3: default mode or menu the picker opens in Menu mode in ios and android

@codecov

codecov Bot commented Aug 6, 2019

Copy link
Copy Markdown

Codecov Report

Merging #56 into develop will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           develop       #56   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           57        60    +3     
  Lines          310       384   +74     
  Branches        32        36    +4     
=========================================
+ Hits           310       384   +74     
Impacted Files Coverage Δ
src/index.tsx 100.00% <ø> (ø)
src/components/Picker/Picker.android.tsx 100.00% <100.00%> (ø)
src/components/Picker/Picker.tsx 100.00% <100.00%> (ø)
src/components/Picker/index.tsx 100.00% <100.00%> (ø)
src/components/index.tsx 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 a85ea2b...27d5f29. Read the comment docs.

Comment thread bluebase/storybook-native/storybook/storyLoader.js
Comment thread package.json Outdated
Comment thread src/components/Picker/index.tsx
Comment thread src/index.tsx
Comment thread tsconfig.json
Comment thread src/components/Picker/Picker.android.tsx Outdated
selectedValue: string,
styles: PickerStyles,
label: string,
mode: 'modal' | 'actionSheet';

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.

Make actionsheet all lower case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

resolved

container: {
backgroundColor: '#fff',
borderColor: '#ddd',
borderTopWidth: 0.5,

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.

Why is this hard coded

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the colors are hard coded because android does not have any actionsheet effect so just to make it applicable i have made in modal and poped up through bottom

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.

So what happens on dark theme? We cannot hardcode Colors. Or themeing will not work


static defaultStyles = (_theme: Theme) => ({
container: {
backgroundColor: '#fff',

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.

Why are colours hard coded

@artalat

artalat commented Aug 26, 2019

Copy link
Copy Markdown
Contributor

On a deeper look, the implementation is wrong. This should be the usage:

<Picker
  selectedValue={this.state.language}
  style={{height: 50, width: 100}}
  onValueChange={(itemValue, itemIndex) =>
    this.setState({language: itemValue})
  }>
  <Picker.Item label="Java" value="java" />
  <Picker.Item label="JavaScript" value="js" />
</Picker>

Remember, the api cannot change from the official version:

https://facebook.github.io/react-native/docs/picker

@adnan1naeem

Copy link
Copy Markdown
Contributor Author

On a deeper look, the implementation is wrong. This should be the usage:

<Picker
  selectedValue={this.state.language}
  style={{height: 50, width: 100}}
  onValueChange={(itemValue, itemIndex) =>
    this.setState({language: itemValue})
  }>
  <Picker.Item label="Java" value="java" />
  <Picker.Item label="JavaScript" value="js" />
</Picker>

Remember, the api cannot change from the official version:

https://facebook.github.io/react-native/docs/picker

On a deeper look, the implementation is wrong. This should be the usage:

<Picker
  selectedValue={this.state.language}
  style={{height: 50, width: 100}}
  onValueChange={(itemValue, itemIndex) =>
    this.setState({language: itemValue})
  }>
  <Picker.Item label="Java" value="java" />
  <Picker.Item label="JavaScript" value="js" />
</Picker>

Remember, the api cannot change from the official version:

https://facebook.github.io/react-native/docs/picker

can u please explain as i have implemented things based on using picker component however we have listItem on modal popup as we discussed

@ammarkhan967 ammarkhan967 added the Feature something new is added label Aug 26, 2019
@artalat

artalat commented Aug 26, 2019

Copy link
Copy Markdown
Contributor

@adnan1naeem it's better if you first finish this: BlueBaseJS/plugin-material-ui#80

@artalat

artalat commented Aug 26, 2019

Copy link
Copy Markdown
Contributor

As for mode="menu" here is a reference implementation: callstack/react-native-paper#603

@adnan1naeem

Copy link
Copy Markdown
Contributor Author

As for mode="menu" here is a reference implementation: callstack/react-native-paper#603

the default mode is the menu mode actually i have already implemented it

@artalat artalat added the enhancement New feature or request label Aug 29, 2019
@ammarkhan967 ammarkhan967 assigned danial24 and unassigned adnan1naeem Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Feature something new is added

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants