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

v1.7.0 - Icon updates #29

Merged
merged 7 commits into from
Aug 30, 2018
Merged

v1.7.0 - Icon updates #29

merged 7 commits into from
Aug 30, 2018

Conversation

sonnyprince
Copy link
Collaborator

@sonnyprince sonnyprince commented Aug 30, 2018 •
edited
Loading

  • Addition of Cash icon
  • Addition of Clock icon
  • Remove path fill from sort amount icon
  • Update Map pin icon to new version

@sonnyprince sonnyprince requested a review from a team as a code owner August 30, 2018 08:28
DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/tools/mapPin.svg Outdated
<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20">
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the width and height attributes as they interfere with resizing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, all of the other tool icons have default fill or black (no fill colour is set), this should follow suit. If you need to change the colour then you can use the colour modifier classes.

DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/general/offer.svg Outdated
@@ -1 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20"><path d="M12.19 8.22a2.08 2.08 0 0 1-.67.74 1.82 1.82 0 0 1-1.14.31h-.05a.42.42 0 0 1-.37-.42c0-1.25-.23-1.94-.58-2.23a.7.7 0 0 0-.08-.06c-.09.96-.39 1.6-.96 2.32l-.22.29c-.43.54-.58.87-.58 1.47 0 1.17.95 1.87 2.47 1.87a2.41 2.41 0 0 0 2.49-2.32c0-.84-.12-1.5-.31-1.97zm-1.13.05c.28-.2.5-.53.62-1.06a.42.42 0 0 1 .65-.24c.16.11.36.36.55.75.28.6.45 1.42.45 2.48A3.24 3.24 0 0 1 10 13.34c-1.93 0-3.3-1.01-3.3-2.7 0-.81.23-1.32.76-1.99l.23-.28c.55-.7.78-1.26.8-2.3a.42.42 0 0 1 .32-.4l.08-.01a1.32 1.32 0 0 1 1.02.32c.51.42.82 1.2.88 2.42.08-.03.18-.07.26-.13zm6.05 4.11c.05-.16-.04-.4-.4-.97-.4-.67-.54-.95-.53-1.35 0-.37.14-.7.44-1.18l.11-.18c.37-.58.47-.8.42-.95-.04-.15-.26-.28-.86-.53l-.04-.02c-.7-.3-.99-.46-1.22-.78-.23-.33-.3-.66-.34-1.4-.04-.7-.1-.95-.23-1.05-.11-.08-.36-.06-1 .1l-.12.02c-.64.15-1 .18-1.38.05-.39-.13-.65-.37-1.12-.94a20.97 20.97 0 0 0-.18-.21c-.3-.35-.47-.49-.6-.49-.14 0-.34.16-.7.59l-.09.1c-.5.57-.73.8-1.12.92-.38.12-.73.07-1.41-.1L6.68 4c-.66-.16-.9-.19-1.03-.1-.12.09-.18.33-.24.99v.06c-.06.72-.13 1.06-.37 1.38-.24.33-.52.47-1.24.76-.63.25-.86.38-.91.54-.05.16.04.4.4.98.4.66.54.94.53 1.34 0 .37-.14.7-.44 1.18l-.11.18c-.37.58-.47.8-.42.95.04.15.26.28.86.53l.04.02c.7.3.99.45 1.22.78.23.33.3.66.34 1.4.04.7.1.95.23 1.05.11.08.36.06 1-.1l.12-.02c.64-.15 1-.18 1.38-.06.39.14.65.38 1.12.95l.18.21c.3.35.47.49.6.49.14 0 .34-.16.7-.59l.09-.1c.5-.57.74-.8 1.12-.92.38-.12.73-.08 1.41.1l.06.01c.66.17.9.2 1.03.1.12-.08.18-.32.24-.98v-.06c.06-.72.13-1.06.38-1.38.23-.33.51-.47 1.23-.76.63-.25.86-.38.91-.54zm-.6 1.32c-.56.22-.77.33-.88.47-.1.15-.16.4-.2.96l-.01.06c-.08.94-.17 1.3-.59 1.6-.41.3-.78.26-1.71.02l-.06-.01c-.53-.14-.78-.17-.96-.11-.17.05-.36.22-.74.67l-.08.1c-.54.62-.86.88-1.36.87-.45 0-.77-.25-1.22-.78l-.18-.22c-.37-.44-.56-.62-.75-.68-.17-.06-.43-.04-.92.08l-.11.02c-.94.22-1.3.25-1.7-.05-.42-.31-.5-.68-.56-1.64-.04-.6-.08-.84-.19-1-.1-.14-.32-.27-.86-.49-.9-.38-1.22-.58-1.38-1.07-.15-.49-.01-.82.51-1.65l.11-.17c.22-.36.32-.58.32-.75 0-.17-.1-.4-.41-.9-.5-.82-.65-1.18-.48-1.68.17-.5.5-.7 1.4-1.05.55-.22.76-.33.87-.47.1-.15.16-.4.2-.96l.01-.06c.08-.94.17-1.3.58-1.6.42-.3.79-.26 1.72-.02l.06.01c.53.14.78.17.95.11.18-.05.37-.22.75-.67l.08-.1c.54-.62.86-.88 1.35-.87.46 0 .78.25 1.23.78l.18.22c.37.44.56.62.75.68.17.06.43.04.92-.08l.11-.02c.94-.22 1.3-.25 1.7.05.42.31.5.68.56 1.64.04.6.08.84.19 1 .1.14.32.26.86.49.9.37 1.22.58 1.38 1.07.15.5.01.82-.51 1.65l-.1.16c-.23.37-.33.59-.33.76 0 .17.1.4.41.9.5.82.65 1.18.48 1.68-.17.5-.5.7-1.4 1.05z"/></svg>
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20"><path fill="#cd390c" d="M12.19 8.22a2.08 2.08 0 0 1-.67.74 1.82 1.82 0 0 1-1.14.31h-.05a.42.42 0 0 1-.37-.42c0-1.25-.23-1.94-.58-2.23a.7.7 0 0 0-.08-.06c-.09.96-.39 1.6-.96 2.32l-.22.29c-.43.54-.58.87-.58 1.47 0 1.17.95 1.87 2.47 1.87a2.41 2.41 0 0 0 2.49-2.32c0-.84-.12-1.5-.31-1.97zm-1.13.05c.28-.2.5-.53.62-1.06a.42.42 0 0 1 .65-.24c.16.11.36.36.55.75.28.6.45 1.42.45 2.48A3.24 3.24 0 0 1 10 13.34c-1.93 0-3.3-1.01-3.3-2.7 0-.81.23-1.32.76-1.99l.23-.28c.55-.7.78-1.26.8-2.3a.42.42 0 0 1 .32-.4l.08-.01a1.32 1.32 0 0 1 1.02.32c.51.42.82 1.2.88 2.42.08-.03.18-.07.26-.13zm6.05 4.11c.05-.16-.04-.4-.4-.97-.4-.67-.54-.95-.53-1.35 0-.37.14-.7.44-1.18l.11-.18c.37-.58.47-.8.42-.95-.04-.15-.26-.28-.86-.53l-.04-.02c-.7-.3-.99-.46-1.22-.78-.23-.33-.3-.66-.34-1.4-.04-.7-.1-.95-.23-1.05-.11-.08-.36-.06-1 .1l-.12.02c-.64.15-1 .18-1.38.05-.39-.13-.65-.37-1.12-.94a20.97 20.97 0 0 0-.18-.21c-.3-.35-.47-.49-.6-.49-.14 0-.34.16-.7.59l-.09.1c-.5.57-.73.8-1.12.92-.38.12-.73.07-1.41-.1L6.68 4c-.66-.16-.9-.19-1.03-.1-.12.09-.18.33-.24.99v.06c-.06.72-.13 1.06-.37 1.38-.24.33-.52.47-1.24.76-.63.25-.86.38-.91.54-.05.16.04.4.4.98.4.66.54.94.53 1.34 0 .37-.14.7-.44 1.18l-.11.18c-.37.58-.47.8-.42.95.04.15.26.28.86.53l.04.02c.7.3.99.45 1.22.78.23.33.3.66.34 1.4.04.7.1.95.23 1.05.11.08.36.06 1-.1l.12-.02c.64-.15 1-.18 1.38-.06.39.14.65.38 1.12.95l.18.21c.3.35.47.49.6.49.14 0 .34-.16.7-.59l.09-.1c.5-.57.74-.8 1.12-.92.38-.12.73-.08 1.41.1l.06.01c.66.17.9.2 1.03.1.12-.08.18-.32.24-.98v-.06c.06-.72.13-1.06.38-1.38.23-.33.51-.47 1.23-.76.63-.25.86-.38.91-.54zm-.6 1.32c-.56.22-.77.33-.88.47-.1.15-.16.4-.2.96l-.01.06c-.08.94-.17 1.3-.59 1.6-.41.3-.78.26-1.71.02l-.06-.01c-.53-.14-.78-.17-.96-.11-.17.05-.36.22-.74.67l-.08.1c-.54.62-.86.88-1.36.87-.45 0-.77-.25-1.22-.78l-.18-.22c-.37-.44-.56-.62-.75-.68-.17-.06-.43-.04-.92.08l-.11.02c-.94.22-1.3.25-1.7-.05-.42-.31-.5-.68-.56-1.64-.04-.6-.08-.84-.19-1-.1-.14-.32-.27-.86-.49-.9-.38-1.22-.58-1.38-1.07-.15-.49-.01-.82.51-1.65l.11-.17c.22-.36.32-.58.32-.75 0-.17-.1-.4-.41-.9-.5-.82-.65-1.18-.48-1.68.17-.5.5-.7 1.4-1.05.55-.22.76-.33.87-.47.1-.15.16-.4.2-.96l.01-.06c.08-.94.17-1.3.58-1.6.42-.3.79-.26 1.72-.02l.06.01c.53.14.78.17.95.11.18-.05.37-.22.75-.67l.08-.1c.54-.62.86-.88 1.35-.87.46 0 .78.25 1.23.78l.18.22c.37.44.56.62.75.68.17.06.43.04.92-.08l.11-.02c.94-.22 1.3-.25 1.7.05.42.31.5.68.56 1.64.04.6.08.84.19 1 .1.14.32.26.86.49.9.37 1.22.58 1.38 1.07.15.5.01.82-.51 1.65l-.1.16c-.23.37-.33.59-.33.76 0 .17.1.4.41.9.5.82.65 1.18.48 1.68-.17.5-.5.7-1.4 1.05z"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the only difference the fill colour? If so I added the icon colour modifiers to handle this i.e. c-icon--orange in this case.

The reasoning behind this was to give the icons a default colour of black and then use the modifier classes to update them.

Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

perfect! i've got rid of that

DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/general/cash.svg Outdated
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20"><path fill="#333" d="M11.25 16.667a2.083 2.083 0 1 1 0-4.167 2.083 2.083 0 0 1 0 4.167zm0-.834a1.25 1.25 0 1 0 0-2.5 1.25 1.25 0 0 0 0 2.5zm-6.258-4.166V17.5H17.5v-5.833H4.992zm-.417-.834h13.342c.23 0 .416.187.416.417v6.667c0 .23-.186.416-.416.416H4.575a.417.417 0 0 1-.417-.416V11.25c0-.23.187-.417.417-.417zm-1.872-1.45l.824 1.41a.417.417 0 0 1-.72.42l-1.033-1.77a.417.417 0 0 1 .15-.57l11.524-6.725a.417.417 0 0 1 .57.15l3.367 5.758a.417.417 0 0 1-.15.57l-2.258 1.317a.417.417 0 1 1-.42-.72l1.898-1.106-2.946-5.039L2.703 9.383zm8.682.41a.417.417 0 0 1-.72-.419 1.25 1.25 0 1 0-2.163 0 .417.417 0 0 1-.72.418 2.083 2.083 0 1 1 3.603 0z"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace the width and height attributes with viewBox="0 0 20 20.

DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/general/clock.svg Outdated
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20"><path fill="#333" d="M12.795 12.205a.417.417 0 1 1-.59.59l-2.622-2.622V6.25a.417.417 0 1 1 .834 0v3.577l2.378 2.378zM10 17.5a7.5 7.5 0 1 1 0-15 7.5 7.5 0 0 1 0 15zm0-.833a6.667 6.667 0 1 0 0-13.334 6.667 6.667 0 0 0 0 13.334z"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace the width and height attributes with viewBox="0 0 20 20.

DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/tools/sortAmount.svg Outdated
<svg width="16" height="10" viewBox="0 0 16 10" xmlns="http://www.w3.org/2000/svg"><path fill="#266abd" d="M.44.89a.44.44 0 0 1 0-.89h15.12a.44.44 0 0 1 0 .89H.44zm0 4.44a.44.44 0 0 1 0-.89h9.78a.44.44 0 1 1 0 .9H.44zm0 4.45a.44.44 0 0 1 0-.9H4.9a.44.44 0 1 1 0 .9H.44z" fill="#000" fill-rule="nonzero"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace the width and height attributes with viewBox="0 0 20 20.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, when trying to view this SVG it says it is invalid?

@sonnyprince
Copy link
Collaborator Author

@DamianMullins - all updated :) 👍

DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/general/cash.svg Outdated
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20"><path fill="#333" d="M11.25 16.667a2.083 2.083 0 1 1 0-4.167 2.083 2.083 0 0 1 0 4.167zm0-.834a1.25 1.25 0 1 0 0-2.5 1.25 1.25 0 0 0 0 2.5zm-6.258-4.166V17.5H17.5v-5.833H4.992zm-.417-.834h13.342c.23 0 .416.187.416.417v6.667c0 .23-.186.416-.416.416H4.575a.417.417 0 0 1-.417-.416V11.25c0-.23.187-.417.417-.417zm-1.872-1.45l.824 1.41a.417.417 0 0 1-.72.42l-1.033-1.77a.417.417 0 0 1 .15-.57l11.524-6.725a.417.417 0 0 1 .57.15l3.367 5.758a.417.417 0 0 1-.15.57l-2.258 1.317a.417.417 0 1 1-.42-.72l1.898-1.106-2.946-5.039L2.703 9.383zm8.682.41a.417.417 0 0 1-.72-.419 1.25 1.25 0 1 0-2.163 0 .417.417 0 0 1-.72.418 2.083 2.083 0 1 1 3.603 0z"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the fill colour here and apply the c-icon--grey--darkest class to set it to #333.

DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/general/clock.svg Outdated
@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20"><path fill="#333" d="M12.795 12.205a.417.417 0 1 1-.59.59l-2.622-2.622V6.25a.417.417 0 1 1 .834 0v3.577l2.378 2.378zM10 17.5a7.5 7.5 0 1 1 0-15 7.5 7.5 0 0 1 0 15zm0-.833a6.667 6.667 0 1 0 0-13.334 6.667 6.667 0 0 0 0 13.334z"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the fill colour here and apply the c-icon--grey--darkest class to set it to #333.

DamianMullins
DamianMullins reviewed Aug 30, 2018
View reviewed changes
src/img/icons/tools/sortAmount.svg Outdated
<svg width="16" height="10" viewBox="0 0 16 10" xmlns="http://www.w3.org/2000/svg"><path d="M.44.89a.44.44 0 0 1 0-.89h15.12a.44.44 0 0 1 0 .89H.44zm0 4.44a.44.44 0 0 1 0-.89h9.78a.44.44 0 1 1 0 .9H.44zm0 4.45a.44.44 0 0 1 0-.9H4.9a.44.44 0 1 1 0 .9H.44z" fill-rule="nonzero"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the width and height attributes.

@sonnyprince sonnyprince requested review from a team August 30, 2018 12:13
@sonnyprince sonnyprince merged commit 08b4fb3 into master Aug 30, 2018
@sonnyprince sonnyprince deleted the 1.6.0-icon-updates branch August 30, 2018 12:14
ashleynolan
ashleynolan reviewed Aug 30, 2018
View reviewed changes
src/img/icons/tools/mapPin.svg
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20">
<path fill-rule="nonzero" d="M10 10.833a2.5 2.5 0 1 1 0-5 2.5 2.5 0 0 1 0 5zM10 10a1.667 1.667 0 1 0 0-3.333A1.667 1.667 0 0 0 10 10zm4.716-6.485a6.633 6.633 0 0 1 2.034 4.657c0 2.256-1.23 4.621-3.268 6.974a25.286 25.286 0 0 1-3.228 3.101.417.417 0 0 1-.508 0 23.028 23.028 0 0 1-1.014-.857 25.286 25.286 0 0 1-2.214-2.244C4.48 12.793 3.25 10.428 3.25 8.163A6.628 6.628 0 0 1 10 1.667a6.628 6.628 0 0 1 4.716 1.848zM10.711 16.77c.73-.655 1.46-1.385 2.14-2.17 1.92-2.215 3.066-4.418 3.066-6.42a5.795 5.795 0 0 0-5.909-5.68 5.806 5.806 0 0 0-4.147 1.616 5.79 5.79 0 0 0-1.778 4.056c0 2.01 1.146 4.213 3.065 6.428a24.463 24.463 0 0 0 2.845 2.776c.226-.174.463-.377.718-.606z"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

More a question for my own knowledge. What does fill-rule do for these SVGs? Seems some SVGs have them and others don't

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashleynolan ashleynolan ashleynolan left review comments

@simonsandersje simonsandersje simonsandersje approved these changes

@DamianMullins DamianMullins DamianMullins approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sonnyprince @DamianMullins @ashleynolan @simonsandersje