feat(starred-count): show starred count#271
feat(starred-count): show starred count#271housseindjirdeh merged 18 commits intogitpoint:masterfrom
Conversation
andrewda
left a comment
There was a problem hiding this comment.
Quick look at the code seems good! Thanks so much!
src/api/index.js
Outdated
| const ENDPOINT = `https://api.github.com/users/${owner}/starred?per_page=1`; | ||
|
|
||
| const requestPromise = new Promise((resolve, reject) => { | ||
| const req = new XMLHttpRequest(); |
There was a problem hiding this comment.
You can use fetch instead, quick example:
fetch('https://api.github.com/users/lex111/starred?per_page=1').then(function (response) {
console.log(response.headers.get('Link'));
});| {starCount} | ||
| </Text> | ||
| <Text style={styles.unitText}> | ||
| {translate('common.star', language)} |
There was a problem hiding this comment.
It is better to use the plural (stars), by analogy with other (repositories, followers).
src/locale/languages/fr.js
Outdated
| }, | ||
| common: { | ||
| bio: 'BIO', | ||
| star: 'étoiles', |
There was a problem hiding this comment.
If look at the key starsTitle, then the stars are translated as "favoris".
There was a problem hiding this comment.
yes, it should be favoris. Merci beaucoup Mr @lex111 !
src/locale/languages/nl.js
Outdated
| }, | ||
| common: { | ||
| bio: 'BIO', | ||
| star: 'Sterren', |
There was a problem hiding this comment.
And here it will be simple - Stars (again, if look at the key starsTitle)
|
5ab1671 reduce the left and right margin of the badge in
|
src/user/user.type.js
Outdated
| export const GET_FOLLOWING = createActionSet('GET_FOLLOWING'); | ||
| export const SEARCH_USER_REPOS = createActionSet('SEARCH_USER_REPOS'); | ||
| export const CHANGE_FOLLOW_STATUS = createActionSet('CHANGE_FOLLOW_STATUS'); | ||
| export const GET_STAR_COUNT = createActionSet('CHANGE_FOLLOW_STATUS'); |
| case GET_STAR_COUNT.PENDING: | ||
| return { | ||
| ...state, | ||
| starCount: ' ', |
There was a problem hiding this comment.
Probably it is possible to remove, after all in initialStore already there is this key?
There was a problem hiding this comment.
@lex111 I have decided to remove it from initialStore, and only add to it when GET_STAR_COUNT.PENDING and GET_AUTH_STAR_COUNT.PENDING are respectively dispatched.
There was a problem hiding this comment.
Well, I would still put it in initialStore, and then it could only be added to GET_STAR_COUNT.SUCCESS, but I'm ok.
src/locale/languages/nl.js
Outdated
| }, | ||
| common: { | ||
| bio: 'BIO', | ||
| stars: 'Stars', |
There was a problem hiding this comment.
Dutch should be: 'Sterren'
There was a problem hiding this comment.
Thanks for your help! Will change the translation
RolfKoenders
left a comment
There was a problem hiding this comment.
Looks good! I added as a comment the dutch translation for 'stars'.
|
Overall this looks so solid :O :O Thanks so much @adrianhartanto0, there are literally just the few things @RolfKoenders and @lex111 noticed that I can see will need changing but otherwise this should be solid to merge 🎉 |
housseindjirdeh
left a comment
There was a problem hiding this comment.
Thanks a bunch @adrianhartanto0 🎉 🎉 🎉
This is to close #129. I followed the suggestion made by @RolfKoenders, and used
XMLHttpRequestinstead offetchto retrieve the star count.I have also included a dutch and french translation for the "star" word using Google translate. @RolfKoenders @machour, could you please verify that the translated word is correct?