The Wayback Machine - https://web.archive.org/web/20201218002303/https://github.com/gitpoint/git-point/pull/739
Skip to content
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(user): split repositories list by type #739

Open
wants to merge 4 commits into
base: master
from

Conversation

@Arjun-sna
Copy link
Contributor

@Arjun-sna Arjun-sna commented Feb 26, 2018

Question Response
Version? v1.4.1
Devices tested? Oneplus 5
Bug fix? no
New feature? yes
Includes tests? no
All Tests pass? yes
Related ticket?

Screenshots Auth user

Before After After
before after after

Screenshots user

Before After
before after

Description

Added categories to repo list screen

Copy link
Member

@machour machour left a comment

This is great @Arjun-sna !

I left you some remarks, my biggest concern being that the button group won't play nice with longer texts when using translations .. not really sure how to properly handle it :/

'user.repositoryList.memberReposButton',
locale
),
]

This comment has been minimized.

@machour

machour Feb 26, 2018
Member

Could this be computed outside the JSX and condensed?

const buttons = ['common','buttons'];
if (authUser.login === currentuser.login) {
   buttons.push('other');
   buttons.push('buttons');
}

headerRight: (
<Icon
name="search"
color={colors.primaryDark}

This comment has been minimized.

@machour

machour Feb 26, 2018
Member

personal taste: maybe greyDark would be better (in find the loop "too black")

</SearchContainer>
</SearchBarWrapper>
) : (
<StyledButtonGroup

This comment has been minimized.

@machour

machour Feb 26, 2018
Member

English labels fits right in, but I'm afraid other translations won't play as nice.. 🤔
Try with french labels for example: "Tous, Propriétaire, Membre, Privé, Public"

@machour machour changed the title Repo list categories feat(user): split repositories list by type Feb 26, 2018
@machour
Copy link
Member

@machour machour commented Feb 26, 2018

(In order to fix the Travis build, you have to add the english sentences in all the translations files.
You can use the translations I provided for French, leave other as English until a native speaker does the translation)

Arjun-sna added 2 commits Feb 27, 2018
add default texts for repo type titles for other languages
@Arjun-sna
Copy link
Contributor Author

@Arjun-sna Arjun-sna commented Feb 27, 2018

@machour tried to fix longer texts with ellipsizeMode. Please check the updated screenshot under auth user. Also worked on other remarks you have provided.

@coveralls
Copy link

@coveralls coveralls commented Feb 27, 2018

Coverage Status

Coverage remained the same at 45.125% when pulling 7e98e9d on Arjun-sna:repo_list_categories into f80885f on gitpoint:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.