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

Issue #4191 - Profile page empty kudos state #4192

Merged
merged 10 commits into from
May 13, 2019

Conversation

MaxStalker
Copy link
Contributor

@MaxStalker MaxStalker commented Apr 12, 2019

Description

SVG images inside empty kudos state have incorrect text alignment and incorrect font. Besides it doesn't allow option for translation.

Checklist
  • Read and conforms to the contributor guidelines
  • Read and conforms to the style guidelines
  • Linter status: 100% pass
  • Added relevant tests
  • Tests pass and code coverage has not decreased
  • Changes don't break existing behavior
  • Commit message follows commit guidelines
  • Completed: Code review by core team and any requested changes
Affected core subsystem(s)

UI

Refers/Fixes

Fixes #4191

Testing and Sign-off

This PR was tested on different screen sizes to ensure proper responsiveness. Video of the process can be found here:
https://www.loom.com/share/2844894650364b329f36b06cb73cfb32

Contributor
  • Read and followed the Contributor Guidelines
  • Tested all changes locally
  • Verified existing functionality
  • Ran make test and everything passed!
Reviewer
  • Affirm contributor guidelines have been followed and requested changes made
  • CI tests and linting pass
  • No conflicts (migrations, files, etc)
  • Regression tested against staging or local deployment
Funder
  • Validated requested changes were made to specification
  • Bounty payout released to the contributor

@codecov
Copy link

codecov bot commented Apr 12, 2019

Codecov Report

Merging #4192 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4192      +/-   ##
==========================================
- Coverage   30.19%   30.16%   -0.04%     
==========================================
  Files         209      209              
  Lines       16790    16790              
  Branches     2249     2249              
==========================================
- Hits         5070     5064       -6     
- Misses      11527    11533       +6     
  Partials      193      193
Impacted Files Coverage Δ
app/dashboard/embed.py 28.16% <0%> (-3.45%) ⬇️

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 d47f8e6...35d42fa. Read the comment docs.

@octavioamu octavioamu self-requested a review April 12, 2019 18:17
@thelostone-mc
Copy link
Member

@MaxStalker Could you add screenshot/recording ?

@MaxStalker
Copy link
Contributor Author

@thelostone-mc there was a link in description to a video :)
https://www.loom.com/share/2844894650364b329f36b06cb73cfb32

@octavioamu
Copy link
Contributor

Maybe we can keep the draw line color in #405366 or there is some reason about that change?

image

Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

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

Look nice Max, just have that question

@MaxStalker
Copy link
Contributor Author

@octavioamu currently there is animation on those hexagons (and icons inside of them) Do we want to preserve it? Or we can simple set the color and make them static.
If we want to keep animations - then we need to inline SVGs. Otherwise there is no way to use css animation on vector elements. If we decide to scrap animation and just leave them at some set color - then we can optimize the hell out of them and put in image resource folder.
Your call :)

@octavioamu
Copy link
Contributor

@octavioamu currently there is animation on those hexagons (and icons inside of them) Do we want to preserve it? Or we can simple set the color and make them static.
If we want to keep animations - then we need to inline SVGs. Otherwise there is no way to use css animation on vector elements. If we decide to scrap animation and just leave them at some set color - then we can optimize the hell out of them and put in image resource folder.
Your call :)

Check the color I sent I think you will get what I'm saying, is not about the exagon color, just the illustration inside in my local is black instead of that darkblue
Example, the line of the bag here #000 :
image

@MaxStalker
Copy link
Contributor Author

@octavioamu ah, I see now. I've made proper changes together with svgo optimization

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

lgtm !
@octavioamu ?

@thelostone-mc thelostone-mc merged commit 465fdae into gitcoinco:master May 13, 2019
@thelostone-mc
Copy link
Member

Thanks @MaxStalker ^_^

@octavioamu
Copy link
Contributor

@MaxStalker this got merged but I'm having 2 issues
1
static is not loaded in kudos_hex_container.html
2
image

@thelostone-mc was working fine in your local?

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.

Design - Empty State for kudos on profile page
3 participants