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

Screens/social profile screen #56

Open
wants to merge 4 commits into
base: examples
Choose a base branch
from

Conversation

samcyn
Copy link
Contributor

@samcyn samcyn commented Nov 11, 2018

Social Profile Screen
I am done with this screen and would like some reviews on the work done. I enjoyed using Galio a lot. If not for the issues I had picking out some detailed measurements from the designs I would be done in a day or so. Well check out what I got and let me know what you think.
Regards.

@palingheorghe
Copy link
Collaborator

Related to #31

Copy link
Collaborator

@palingheorghe palingheorghe left a comment

Choose a reason for hiding this comment

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

Hi,

Nice! This looks awesome! Could you expand on the issues you encountered?
You've done a great job! I'm glad you found it easy and cool to work with Galio ❤️.

When I first opened your screen's file I noticed there are 54 eslint errors, could you fix them?

Thanks,
Alin 🤙🏽

src/screens/SocialProfile.js Outdated Show resolved Hide resolved
@hetmann
Copy link
Collaborator

hetmann commented Nov 12, 2018

@samcyn a quick check for lint is to run npm run lint src/screens/SocialProfile.js
To auto fix them run npm run lint src/screens/SocialProfile.js --fix and some of the warning/errors must be done manually

@samcyn
Copy link
Contributor Author

samcyn commented Nov 15, 2018

update: All linting errors have been fixed. Thanks @hetmann and @palingheorghe for the review.

@samcyn
Copy link
Contributor Author

samcyn commented Nov 15, 2018

Hi one more change. I added COLOR_DARK_GREY='#444444' as suggested by @hetmann. And that should be all for now. Thanks guys.

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

Successfully merging this pull request may close these issues.

3 participants