Skip to content

style: Update interface of badges in repository component#709

Merged
machour merged 2 commits intogitpoint:masterfrom
rkpasia:ft-badge-layout
Jan 31, 2018
Merged

style: Update interface of badges in repository component#709
machour merged 2 commits intogitpoint:masterfrom
rkpasia:ft-badge-layout

Conversation

@rkpasia
Copy link
Copy Markdown
Contributor

@rkpasia rkpasia commented Jan 30, 2018

Question Response
Version? v1.4.1
Devices tested? iPhone 6 (Emulator)
Bug fix? no
New feature? no
Includes tests? no
All Tests pass? yes
Related ticket?

Screenshots

Before After
before after

Description

Hello! In this short commit I changed the styles associated with the badges in the repository component. I wasn't happy with the previous interface, so I tried to play with it and the results can be seen in the screenshots :)

I haven't created an issue or discussed with anyone else about this before doing it. So if it isn't accepted I understand, it just seemed a good way to start contributing to the project :)

@coveralls
Copy link
Copy Markdown

coveralls commented Jan 30, 2018

Coverage Status

Coverage remained the same at 44.074% when pulling aa60465 on rkpasia:ft-badge-layout into 2111d74 on gitpoint:master.

@machour
Copy link
Copy Markdown
Member

machour commented Jan 30, 2018

Hey, this is a fantastic way to start contributing, what a nice UI update 🎉 🎉

Left you some minor changes requests to stick to our coding habits.

Could you also add yourself as a contributor using yarn contributor:add ?

Thank you again for your PR!

marginLeft: 27,
marginRight: 27,
borderWidth: 0.5,
badge_view: {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badgeView would be me consistent with our code style

},
badge: {
color: colors.white,
fontWeight: 'bold',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ...fonts.fontPrimaryBold instead

@housseindjirdeh
Copy link
Copy Markdown
Member

This is so great @rkpasia 😍 . I knew those tags could use a little bit of love, but couldn't place my finger on what exactly it needed to look a little better.

Aside from the tiny things @machour, this looks great. Also, please don't forget to add yourself as a contributor with yarn contributors:add!!

@rkpasia
Copy link
Copy Markdown
Contributor Author

rkpasia commented Jan 30, 2018

@machour thanks for the feedback! 😄 In the last commit I've updated the code for consistency.

@housseindjirdeh Thanks for the great words 😄I've added myself to the contributors list! Forgot about it ahah

I'll reach out in gitter about the possibility to further get involved in the project 😄

@machour machour merged commit 3d793a5 into gitpoint:master Jan 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants