-
Notifications
You must be signed in to change notification settings - Fork 734
Feat/new chips input #1681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/new chips input #1681
Conversation
…w more control with fieldStyle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, but seems like we remove some features, is it intended?
Also, there're some commented lines that I think we can remove..
const styles = StyleSheet.create({ | ||
container: {} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed :)
src/incubator/ChipsInput/index.tsx
Outdated
chips?: ChipProps[]; | ||
defaultChipProps?: ChipProps; | ||
onChange?: (chips: ChipProps[], changeReason: ChipsInputChangeReason, updatedChip: ChipProps) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add descriptions for these props as well
key={index} | ||
customValue={index} | ||
// resetSpacings | ||
// paddingH-s2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept it for now so I'll have a quick reference in case there is something missing while we're migrating.
I added a TODO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool,
There're few redundant imports, so you can remove them and merge :)
import _ from 'lodash'; | ||
import React, {Component} from 'react'; | ||
import {StyleSheet} from 'react-native'; | ||
import {View, Text, Card, TextField, Button, Colors, Incubator} from 'react-native-ui-lib'; //eslint-disable-line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There're few redundant imports here..
Description
Reimplement new ChipsInput based on Incubator.TextField (under Incubator)
Changelog
New ChipsInput component based on our new TextField implementation