-
Notifications
You must be signed in to change notification settings - Fork 11
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
stack navigator and search screen #114
Conversation
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.
Great effort overall!
Yes, as an exported enum. Use hex where possible.
… On 18 Sep 2020, at 14:56, ਸਹਜਪ੍ਰੀਤ ਸਿੰਘ ***@***.***> wrote:
@saihaj commented on this pull request.
In src/components/Search.tsx:
> flexDirection: 'row',
+ borderRadius: 10,
+ backgroundColor: 'rgb(220,219,224)',
For now should I move this to a src/theme/colors.js?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
ae38b64
to
8545b61
Compare
react-native-safe-area context adds to much and we lose control over changing color of status bar. Check slack thread for screenshots https://shabados.slack.com/archives/CD41G8413/p1600273582403700 Resource: https://stackoverflow.com/a/52458846/11321732
@@ -21,10 +31,10 @@ type SearchBarProps = { | |||
|
|||
const SearchBar = ( { handleTextChanges }: SearchBarProps ) => ( | |||
<View style={styles.searchBar}> | |||
<Icon name="magnify" size={25} /> | |||
<Icon name="magnify" size={25} style={styles.centered} /> | |||
<TextInput | |||
placeholder="Koj" |
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.
@bhajneet do we want to bake in internationalisation from the start? This would be an example of where to start.
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.
Related #39
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.
Yes absolutely, how can I help?
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 think search screen cannot be a candidate yet since we will only support Punjabi search. Having that screen internationalized won’t make much sense unless we figure out how to convert search into languages we support with internationalization.
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.
The placeholder can be "Search" or "Khoj" or whatever language is chosen.
…t bottom Co-authored-by: Harjot Singh <contact@harjot.me>
Co-authored-by: Harjot Singh <contact@harjot.me>
https: //github.com/conventional-changelog/commitlint/tree/master/@commitlint/config-conventional#body-max-line-length Co-Authored-By: Harjot Singh <contact@harjot.me>
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.
Very happy with this, good work!
Summary of PR
Time spent on PR
6 hours
Linked issues
Building block PR for #98 and eventually #96
Reviewers
@Harjot1Singh