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

Clickable bounty hunters #1682 #1688

Merged
merged 2 commits into from
Jul 16, 2018

Conversation

olafghanizadeh
Copy link

@olafghanizadeh olafghanizadeh commented Jul 11, 2018

refs: #1682

  • Make bounty hunters clickable

  • Test for responsiveness

  • Switch assets to SVG and text instead of image

  • Recreate HTML with bootstrap

@thelostone-mc
Copy link
Member

thelostone-mc commented Jul 11, 2018

Good for review ?
And if you replacing images with svgs , let's delete the older ones

@codecov
Copy link

codecov bot commented Jul 11, 2018

Codecov Report

Merging #1688 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1688      +/-   ##
==========================================
+ Coverage   29.15%   29.16%   +<.01%     
==========================================
  Files         134      134              
  Lines       10062    10065       +3     
  Branches     1319     1319              
==========================================
+ Hits         2934     2935       +1     
- Misses       7023     7025       +2     
  Partials      105      105
Impacted Files Coverage Δ
app/dashboard/management/commands/sync_listener.py 0% <0%> (ø) ⬆️
app/app/urls.py 89.74% <0%> (ø) ⬆️
app/retail/views.py 36.77% <0%> (ø) ⬆️
app/dashboard/views.py 14.15% <0%> (+0.1%) ⬆️

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 c398797...65d0a5b. Read the comment docs.

@olafghanizadeh
Copy link
Author

@thelostone-mc Need to clarify what @owocki wants to happen when you click the avatars.

@owocki
Copy link
Contributor

owocki commented Jul 11, 2018

one piece of feedback: can we make hte distance between these two lines less? http://bits.owocki.com/0e2g3P1r0J0w/Screen%20Shot%202018-07-11%20at%2010.25.32%20AM.png

responsiveness looks good.

maybe when you click, we can just add a slight color mask (#111 instead of #000? ) until you release your click?

@thelostone-mc
Copy link
Member

@owocki That seems confusing in my head! Wouldn't users end up waiting longer to see something happen 🤔

@owocki
Copy link
Contributor

owocki commented Jul 13, 2018

ok im not married to the clickability idea if you guys dont think its a good idea

@olafghanizadeh
Copy link
Author

How should we proceed here?

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.

So far, changes look good to me. To reiterate what @thelostone-mc said... we should delete assets that are replaced and update references the new assets as appropriate.

Thanks for squashing the SVGs!

@thelostone-mc
Copy link
Member

@olafghanizadeh just leave them as non clickable :P

@owocki owocki merged commit ec63a70 into gitcoinco:master Jul 16, 2018
mbeacom added a commit that referenced this pull request Jul 16, 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.

5 participants