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

feat: trim icons and transition SVG path elements #3

Merged
merged 9 commits into from
Mar 6, 2018
Merged

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Feb 16, 2018

Just trimming the icons and adding a new one 😄

/cc @olizilla

@hacdias hacdias requested a review from olizilla February 16, 2018 00:21
@hacdias
Copy link
Member Author

hacdias commented Feb 16, 2018

Just pushed a commit to put a transition into SVG paths.

@hacdias
Copy link
Member Author

hacdias commented Feb 16, 2018

Don't merge this yet @olizilla 😄

@olizilla olizilla added the WIP label Feb 16, 2018
@hacdias
Copy link
Member Author

hacdias commented Feb 16, 2018

@olizilla now using the trimmed icons. I think it's ready to merge 😄

ipfs.css Outdated
@@ -369,7 +369,8 @@ See: https://github.com/ipfs-shipyard/ipfs-css

.ipfs-gradient-0 { background-image: linear-gradient(to top, #041727 0%,#043b55 100%); }

.transition-all {
.transition-all,
.transition-all path {
Copy link
Member

Choose a reason for hiding this comment

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

Is this change related? What does it do?

Copy link
Member Author

Choose a reason for hiding this comment

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

It also applies the transition to the SVG paths

@hacdias hacdias changed the title feat: trim icons feat: trim icons and transition SVG path elements Feb 20, 2018
@olizilla
Copy link
Member

@hacdias we don't need to add path to all the things.

http://jsbin.com/celezal/5/edit?html,css,output

The fill and stroke styles follow the cascade as long as nothing overrides them. The svgs need another pass over them to remove the inline styles, and unneeded classes and ids.

Of note, being able to just set a single fill or stroke colour and have it sensibly change the colour of an svg is depends on the construction of the svg. We'll need to eyeball them all, which sounds like we need an svg preview page. I can take a look at that later if your deep in desktop code.

@hacdias
Copy link
Member Author

hacdias commented Feb 21, 2018

@olizilla thanks for noticing. I thought I had tried and it didn't work. Probably I got something wrong.

I just cleaned the SVGs by removing the classes and IDs that weren't needed and removed the fill attribute so now they're black by default unless someone sets their color (which is the intended use I think).

This is the first week of my uni semester so I'm still getting on a new routine 😄 and I'll be a bit less active this week. Although, I won't continue Desktop until this PR is finished.

I'm now trying to finish js-ipfs#1230 and trying to finish some homework.

Do you want to do the Preview Page in a separate PR or still here?

@olizilla
Copy link
Member

Enjoy uni! The svg preview page can wait. I'll have a quick look at the svgs and get this merged.

@hacdias
Copy link
Member Author

hacdias commented Feb 21, 2018

Thanks @olizilla 😄

@hacdias
Copy link
Member Author

hacdias commented Feb 25, 2018

Any developments on this? 😃

@hacdias hacdias mentioned this pull request Feb 28, 2018
1 task
@hacdias
Copy link
Member Author

hacdias commented Feb 28, 2018

Ping @alanshaw @olizilla 😄

@olizilla
Copy link
Member

olizilla commented Mar 5, 2018

@hacdias looking good. There are a few icons missing... i guess some of them don't make sense as glyph or stroke versions, but I just wanted to check with you if you had more that you were processing or if they just don't exist yet:

screenshot 2018-03-05 17 56 26

cc: @akrych

@akrych
Copy link

akrych commented Mar 5, 2018

Hello:) yes, they are correct. I didn't make glyph version for all

@hacdias
Copy link
Member Author

hacdias commented Mar 5, 2018

@olizilla cool! You should commit that page too 😄

Copy link
Member

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

👍

@olizilla olizilla merged commit 7d140cb into master Mar 6, 2018
@olizilla olizilla deleted the trim branch March 6, 2018 12:34
@hacdias
Copy link
Member Author

hacdias commented Mar 6, 2018

Cool! Thanks @olizilla! Could you ping me when you publish a new version, please? 😄

@olizilla
Copy link
Member

olizilla commented Mar 6, 2018

@hacdias ipfs-css@0.3.0 is a thing now!

@hacdias
Copy link
Member Author

hacdias commented Mar 6, 2018

Thank yooou!

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.

4 participants