feat(*): status for following users/starring repos #260

reyhansofian · 2017-08-16T06:13:38Z

Issue #159

Add Starred status below Stars on repo screen
Add Following status below Following on user screen

Preview

andrewda

Wow this is awesome! Could you do the same thing for watchers? Looks like you might need to update your branch to get the latest watcher code.

lex111 · 2017-08-16T07:47:34Z

Can make a smaller font? And/or make a clarification like in Twitter, i.e. instead of "following" show "follows you", if he follows you? Similarly do in "Following", but there will be "following you" if you follow him?

sasongkoadi · 2017-08-16T08:05:15Z

Can I give any suggest for following @lex111 ??

reyhansofian · 2017-08-16T09:35:26Z

@andrewda sure, I'll update my branch and add to watcher as well
@lex111 I'll take a look on the data again. I'm sure there's followers stuff on the store. Will update
@sasongkoadi feel free to give any suggestion 😄

sasongkoadi · 2017-08-16T12:40:18Z

Can I edit following, and starring style, for more looks good? @reyhansofian

reyhansofian · 2017-08-16T12:42:16Z

oh wait, I'm still working on it @sasongkoadi
I'll let you know when it's pushed 😄

reyhansofian · 2017-08-16T12:46:19Z

Updated User Status

Updated Repo Status

reyhansofian · 2017-08-16T12:48:48Z

@sasongkoadi please let me know if you still want to add something to this PR

reyhansofian · 2017-08-16T14:48:30Z

Added 'Follows You' Status

lex111 · 2017-08-16T16:00:14Z

@reyhansofian It looks cool indeed! 👍 Although I'm not sure if this fits into the design, I would add an top margin before the statuses. Because of the rounded corners, it looks like buttons, although they are not them, can they be removed? In any case, thank you, what other ideas, how should be these statuses look like?

andrewda

This just keeps looking nicer and nicer! I agree with @lex111 for more padding on top, too - I'd like to see what that looks like.

andrewda · 2017-08-16T16:04:31Z

src/locale/languages/en.js

@@ -186,6 +186,8 @@ export const en = {
      starsTitle: 'Stars',
      forksTitle: 'Forks',
      forkedFromMessage: 'forked from',
+      starred: 'Starred',
+      watched: 'Watched',


"Watching" is probably better here (for both the key and translation.)

andrewda · 2017-08-16T16:04:50Z

src/components/user-profile.component.js

@@ -156,6 +182,10 @@ export const UserProfile = ({
          <Text style={styles.unitText}>
            {translate('common.following', language)}
          </Text>
+          {isFollower &&
+            <View style={styles.badge}>
+              <Text style={styles.unitStatus}>Follows you</Text>


Needs translation.

reyhansofian · 2017-08-16T16:20:51Z

@lex111 ah you're right! it need to some margin on the top. I'll fix that! Regarding the statuses style, what about make it looks like a label? Just like what Twitter did

@andrewda will update soon!

andrewda · 2017-08-16T16:24:36Z

@reyhansofian Yea that'd look good. I think you could also try reducing the padding within the label to make it look less like a button.

reyhansofian · 2017-08-16T16:26:54Z

@andrewda OK, will do!

reyhansofian · 2017-08-16T17:40:08Z

@lex111 @andrewda updated! please check 😄 let me know if the statuses border still doesn't make sense and we can remove it

andrewda

This looks great!

lex111 · 2017-08-16T18:01:11Z

@reyhansofian not bad! One clarification: will the width of the status increase with the change in the text? Simply in English it looks good, but there will be other languages, and the length will differ and then how will the statuses be displayed, I mean the word breaks is there?

reyhansofian · 2017-08-16T18:15:33Z

@lex111 yeah, it will break into newline. I'm not sure how to do that in RN tho 😞

lex111 · 2017-08-16T18:20:03Z

Then everything is fine? I just thought that there would be no breaks. Although it was cool if the width of the block expanded, or increase more it, than now, do you think?

reyhansofian · 2017-08-16T18:36:01Z

if you're okay with current approach (break into new line instead of expand the container width), then everything is fine 😄 I'll try to make it expanded by tomorrow.

lex111 · 2017-08-16T18:38:58Z

I do not know for sure, but what think @andrewda @housseindjirdeh?

andrewda · 2017-08-16T18:53:05Z

Not sure. I think I kinda prefer expanding the width, but either way works.

housseindjirdeh

This is awesome @reyhansofian and looks so good 😍. I would also align towards expanding the width if text grows but if that's not simple I'm okay with this as well.

Left a few discussion points and one thing that would need changing (removing the duplicate getFollowers call + fixing conflicts). But otherwise it looks good :O

housseindjirdeh · 2017-08-16T20:44:40Z

src/locale/languages/en.js

@@ -157,6 +157,9 @@ export const en = {
    followingList: {
      title: 'Following',
    },
+    followYou: {
+      title: 'Follows you',
+    },


As part of trying to streamline translation updates, could you add a comment to the top of en.js with the commit hash and link to the PR? That way we can sort of be able to tell if translations are not the same between multiple different languages.

CC @machour

And what if you add new phrases for translation into all language files? Then it will be clear what needs to be translated in other languages. In fact, we need to do this, because we get the situation that in the English version we have followYou, and in that case there will be an error (missing value) in other languages.

I just realised that we have France translation. I will add the followYou key there

Can we make it fall back on the English translation if a phrase doesn't exist in a certain language? That would make everything a lot smoother IMO, and make it so we don't have to update every language in small PR's.

@housseindjirdeh no need to do that for the en.js file, this is only for translators to keep some kind of internal record.

+1 on @andrewda request, we need a fallback to english and developers shouldn't have to touch translations files. It used to be the case with the previous system.

@reyhansofian if you do touch the french file, here's the translation: "Vous suit".

housseindjirdeh · 2017-08-16T20:53:18Z

src/user/user.action.js

 export const getUserInfo = user => {
  return dispatch => {
    return dispatch(getUser(user)).then(() => {
      dispatch(getOrgs(user));
      dispatch(
        checkFollowStatus(`https://api.github.com/user/following/${user}`)
      );
+      dispatch(getFollowers(user));


I'm assuming we fire this to get the list of followers as soon as a user navigates to the user profile screen so that we can determine whether we're following them correct? If that's the case, it's also currently being fired in the follower-list.screen so we'll need to remove it there to prevent it being called unnecessarily for the second time.

exactly! I'll remove that action on follower-list.screen. Thanks for pointing out 👍

housseindjirdeh · 2017-08-16T20:53:26Z

.all-contributorsrc

+    },
+    {
+      "login": "reyhansofian",
+      "name": "Reyhan Sofian",


housseindjirdeh · 2017-08-16T20:57:23Z

src/config/colors.js

@@ -12,6 +12,7 @@ export const colors = {
  greyMidLight: '#f6f8fa',
  greyVeryLight: '#fbfbfb',
  lightGreen: '#00FF00',
+  paleGreen: '#359d68',


Curious, do you think paleGreen will look better than green here? Thinking a lighter shade bolded might give it more pop 🎉 but maybe that's just me 😂

naming is hard 😂 how about lighterBoldGreen? 🙈

reyhansofian · 2017-08-18T01:07:22Z

@machour @andrewda added French translation and activate fallback for translation. please check it out

French Translation

Fallbacks

andrewda · 2017-08-18T05:33:57Z

Very nice!

Antoine38660

Nice work dude :)

Antoine38660 · 2017-08-18T14:44:45Z

src/locale/languages/fr.js

@@ -161,6 +161,9 @@ export const fr = {
    followingList: {
      title: 'Following',
    },
+    followYou: {
+      title: 'Vous suit',


housseindjirdeh

Thanks again matey, appreciate this - it looks awesome 🎉

reyhansofian · 2017-08-20T16:55:23Z

thanks guys! 👍

reyhansofian added 3 commits August 16, 2017 13:11

feat(repo): Add starred status below 'Stars' text on repo screen

22dee05

feat(user): Add following status below 'Following' text on user screen

c29f3f7

feat(contributor): add to contributor

056e2d2

andrewda requested changes Aug 16, 2017

View reviewed changes

andrewda changed the title ~~status for following/starring repos~~ feat(*): status for following users/starring repos Aug 16, 2017

fix(*): change style for status

a926245

feat(user): Add follower status

245aa2a

lex111 requested review from Antoine38660, andrewda and housseindjirdeh August 16, 2017 16:00

andrewda requested changes Aug 16, 2017

View reviewed changes

reyhansofian added 2 commits August 17, 2017 00:33

fix(*): change badge style

0704248

fix(lang): add/edit some translation

01ae488

andrewda approved these changes Aug 16, 2017

View reviewed changes

housseindjirdeh requested changes Aug 16, 2017

View reviewed changes

reyhansofian added 6 commits August 16, 2017 21:50

Merge branch 'master' into feature/follow-star-status

25caf94

feat(lang): add hash commit and PR link

4a24d64

fix(*): fix naming and style

b6a98bf

fix(follower): remove getFollowers on follower.list.screen

31ef527

fix(trans): add fr translation

32aed48

feat(trans): activate i18n fallbacks

d869c61

Antoine38660 approved these changes Aug 18, 2017

View reviewed changes

lex111 mentioned this pull request Aug 19, 2017

feat(translation): add dutch translation #268

Merged

Merge branch 'master' into feature/follow-star-status

b151500

housseindjirdeh approved these changes Aug 20, 2017

View reviewed changes

housseindjirdeh merged commit f23cc70 into gitpoint:master Aug 20, 2017

machour mentioned this pull request Sep 13, 2017

Change green number status for following/starring repos #159

Closed

reyhansofian deleted the feature/follow-star-status branch September 16, 2017 23:23

feat(*): status for following users/starring repos #260

feat(*): status for following users/starring repos #260

Conversation

reyhansofian commented Aug 16, 2017

Preview

andrewda left a comment • edited

Choose a reason for hiding this comment

lex111 commented Aug 16, 2017

sasongkoadi commented Aug 16, 2017

reyhansofian commented Aug 16, 2017 • edited

sasongkoadi commented Aug 16, 2017

reyhansofian commented Aug 16, 2017

reyhansofian commented Aug 16, 2017

Updated User Status

Updated Repo Status

reyhansofian commented Aug 16, 2017 • edited

reyhansofian commented Aug 16, 2017

Added 'Follows You' Status

lex111 commented Aug 16, 2017

andrewda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyhansofian commented Aug 16, 2017

andrewda commented Aug 16, 2017

reyhansofian commented Aug 16, 2017

reyhansofian commented Aug 16, 2017 • edited

andrewda left a comment

Choose a reason for hiding this comment

lex111 commented Aug 16, 2017

reyhansofian commented Aug 16, 2017

lex111 commented Aug 16, 2017

reyhansofian commented Aug 16, 2017

lex111 commented Aug 16, 2017

andrewda commented Aug 16, 2017

housseindjirdeh left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyhansofian Aug 17, 2017 • edited

Choose a reason for hiding this comment

reyhansofian commented Aug 18, 2017 • edited

French Translation

Fallbacks

andrewda commented Aug 18, 2017

Antoine38660 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

housseindjirdeh left a comment

Choose a reason for hiding this comment

reyhansofian commented Aug 20, 2017

andrewda left a comment •

edited

reyhansofian commented Aug 16, 2017 •

edited

reyhansofian commented Aug 16, 2017 •

edited

reyhansofian commented Aug 16, 2017 •

edited

housseindjirdeh left a comment •

edited

reyhansofian Aug 17, 2017 •

edited

reyhansofian commented Aug 18, 2017 •

edited