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

Fix: misc ui changes (Gitcoin) #14

Closed
wants to merge 3 commits into from
Closed

Fix: misc ui changes (Gitcoin) #14

wants to merge 3 commits into from

Conversation

tpscrpt
Copy link

@tpscrpt tpscrpt commented Nov 24, 2018

Hey, saw #13 and thought I'd give it a shot.

These fixes should make you happy.

  1. Remove "button" from ListItems to make them clickable links instead
  2. Move the list into the drawer header and restyle the button a little bit

@benoror
Copy link
Member

benoror commented Nov 25, 2018

@JeremiGendron awesome! already approved on Gitcoin, gonna check this out ASAP

@benoror benoror self-requested a review November 25, 2018 16:17
Copy link
Member

@benoror benoror left a comment

Choose a reason for hiding this comment

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

@JeremiGendron just saw your changes, could you please:

  • Revert the <Divider /> and style of the drawerHeader (that wasn't part of the bounty issue)
  • Finish fixing remaining buttons (the circular ones, part of the bounty issue)
  • Fix whitespace/margin left to the main content (also part of the bounty issue)

@tpscrpt
Copy link
Author

tpscrpt commented Nov 25, 2018

@benoror which remaining buttons? The profile + circularIcon button gets unstyled when it's not a button anymore. Contacts? Or UI like (+) buttons.

@benoror
Copy link
Member

benoror commented Nov 25, 2018

@benoror which remaining buttons? The profile + circularIcon button gets unstyled when it's not a button anymore. Contacts? Or UI like (+) buttons.

These buttons:

@tpscrpt
Copy link
Author

tpscrpt commented Nov 25, 2018 via email

@tpscrpt
Copy link
Author

tpscrpt commented Nov 25, 2018

@benoror I'm not sure whether you want Text-only or Icon next to text for those, so I will not continue with the bounty. Submitting clean PR with changes so far.

@tpscrpt
Copy link
Author

tpscrpt commented Nov 25, 2018

@benoror Nevermind! Looks like you took it into your own hands.

@benoror
Copy link
Member

benoror commented Nov 25, 2018

@JeremiGendron I would be glad to commit long term for the project if I like what I see... I need a design-driven React expert.

btw, did you try running the app? You need to sign in using Blockstack: http://blockstack.org

@tpscrpt
Copy link
Author

tpscrpt commented Nov 25, 2018

I'm not an expert, but I can get the job done. I have a lot of time too.

@benoror
Copy link
Member

benoror commented Nov 26, 2018

I'm not an expert, but I can get the job done. I have a lot of time too.

Awesome, please stick with the requirements for now, and feel free to improve other areas as well. Thanks!

@benoror benoror force-pushed the master branch 4 times, most recently from 2bea759 to 966d18f Compare November 30, 2018 16:24
@benoror benoror closed this Dec 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants