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.

Sign up for GitHub

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

Jump to bottom

bounty: review feedback added #711

Merged
merged 1 commit into from
Mar 27, 2018
Merged

bounty: review feedback added #711

merged 1 commit into from
Mar 27, 2018

Conversation

thelostone-mc
Copy link
Member

@thelostone-mc thelostone-mc commented Mar 26, 2018 •
edited
Loading

Description
  • vertical aligned activity section
  • Changed Kill Bounty -> Cancelled Bounty
  • Changed Order of Cancel Bounty & Add contribution

x

screen shot 2018-03-27 at 2 48 21 am

screenshot-2018-3-27 funded issue details gitcoin

Refers/Fixes

@mbeacom mbeacom requested a review from PixelantDesign March 26, 2018 21:15
mbeacom
mbeacom approved these changes Mar 26, 2018
View reviewed changes
Copy link
Contributor

@mbeacom mbeacom left a comment

Choose a reason for hiding this comment

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

Code lgtm - @PixelantDesign ? - This branch is live at: https://stage.gitcoin.co

@mbeacom mbeacom added frontend This needs frontend expertise. ui labels Mar 26, 2018
@codecov
Copy link

codecov bot commented Mar 26, 2018 •
edited
Loading

Codecov Report

Merging #711 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #711   +/-   ##
======================================
  Coverage    34.1%   34.1%           
======================================
  Files          97      97           
  Lines        5453    5453           
  Branches      634     634           
======================================
  Hits         1860    1860           
  Misses       3521    3521           
  Partials       72      72

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff2ab93...998bba7. Read the comment docs.

@PixelantDesign
Copy link
Contributor

@thelostone-mc you are fast!

  • Add contribution should be a secondary button (White button)
  • When there are no tags assigned, we should hide the background of the tags
  • What does the tooltip say on hover over Add contribution?
  • Need more space between these elements, try adding 10px in between months and open, ago and experience, and so on

screen shot 2018-03-26 at 10 32 57 pm

- Can we match inside padding bottom with the top? See below:

screen shot 2018-03-26 at 10 39 01 pm

Thank you!

PixelantDesign
PixelantDesign suggested changes Mar 27, 2018
View reviewed changes
Copy link
Contributor

@PixelantDesign PixelantDesign left a comment

Choose a reason for hiding this comment

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

  • Add contribution should be a secondary button (White button)
  • When there are no tags assigned, hide the background of the tags
  • Need more space between these elements, try adding 10px in between months and open, ago and experience, and so on....
  • Match top and bottom padding on contributors (3) modules

@thelostone-mc thelostone-mc force-pushed the bounty branch 2 times, most recently from 455129c to 2b92a02 Compare March 27, 2018 07:21
@thelostone-mc
Copy link
Member Author

thelostone-mc commented Mar 27, 2018 •
edited
Loading

@PixelantDesign

What does the tooltip say on hover over Add contribution?

Increase the funding for this issue

Add contribution should be a secondary button (White button)
When there are no tags assigned, hide the background of the tags
Need more space between these elements, try adding 10px in between months and open, ago and experience, and so on....
Match top and bottom padding on contributors (3) modules

^ All have been addressed

screen shot 2018-03-27 at 1 52 25 pm

@thelostone-mc
Copy link
Member Author

thelostone-mc commented Mar 27, 2018 •
edited
Loading

@mbeacom all changes added + a bit of refactoring 👍

@PixelantDesign
Copy link
Contributor

@thelostone-mc

Is there no comment count because there arent any comments on this github issue example?

can we increase padding on the USD label? match ETH.

@mbeacom I’d like to look at this one more time on staging before merge.

Thank you.

@thelostone-mc
Copy link
Member Author

thelostone-mc commented Mar 27, 2018 •
edited
Loading

Is there no comment count because there arent any comments on this github issue example?

Yup

can we increase padding on the USD label? match ETH.

Done

screen shot 2018-03-27 at 7 23 28 pm

@PixelantDesign all done

bounty: css changed
998bba7 
- vertical aligned activity section
- Changed Kill Bounty -> Cancelled Bounty
- Changed Order of Cancel Bounty  & Add contribution
- updated tooltip text
- refactored code
- gave breathing space to bounty_info
- changed contribute button to white
@mbeacom
Copy link
Contributor

mbeacom commented Mar 27, 2018

The latest changes are deployed to staging. @PixelantDesign

@PixelantDesign
Copy link
Contributor

looks great! thank you @thelostone-mc

@thelostone-mc
Copy link
Member Author

@mbeacom merge merge :D

@mbeacom
Copy link
Contributor

mbeacom commented Mar 27, 2018

@PixelantDesign Is this good to go?

@mbeacom mbeacom merged commit f001f33 into gitcoinco:master Mar 27, 2018
@PixelantDesign PixelantDesign self-requested a review March 27, 2018 17:53
PixelantDesign
PixelantDesign reviewed Mar 27, 2018
View reviewed changes
Copy link
Contributor

@PixelantDesign PixelantDesign left a comment

Choose a reason for hiding this comment

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

looks good

@owocki
Copy link
Contributor

owocki commented Mar 27, 2018

you are all ⭐️ s

@gitcoinco gitcoinco deleted a comment from darkdarkdragon Jun 7, 2018
@gitcoinco gitcoinco deleted a comment from darkdarkdragon Jun 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PixelantDesign PixelantDesign PixelantDesign requested changes

@mbeacom mbeacom mbeacom approved these changes

Assignees
No one assigned
Labels
frontend This needs frontend expertise.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thelostone-mc @PixelantDesign @mbeacom @owocki