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

Add Canonical #3184

Merged
merged 3 commits into from Jun 9, 2020
Merged

Conversation

gizmecano
Copy link
Contributor

@gizmecano gizmecano commented Jun 6, 2020

canonical

Issue: Request: Canonical #3148

Checklist

  • I updated the JSON data in _data/simple-icons.json
  • I optimized the icon with SVGO or SVGOMG
  • The SVG viewbox is 0 0 24 24

Description

Note I do not check I optimized the icon with SVGO or SVGOMG because I usually operate with svgcleaner that generally does the optimization job well. Let me know if you think there seems to be a significant difference in code optimization.

I used vector files from Canonical symbol set for web and added color #77216F as suggested by @ericcornelissen in his comment.

@gizmecano
Copy link
Contributor Author

As there are indeed significant difference in code optimization, I have finally updated the icon with svgomg online tool.

@ericcornelissen ericcornelissen added the new icon Issues or pull requests for adding a new icon label Jun 6, 2020
@PeterShaggyNoble
Copy link
Member

Thanks for taking this one on, @gizmecano 👍

The path looks spot on to me, it just needs to be centred on the canvas. Also, using canonical-symbol-set-web\SVG\canonical_aubergine_hex.svg from the "Canonical symbol set for web" ZIP, I get #772953 for the colour - which file did you source the colour from, @ericcornelissen?

@PeterShaggyNoble PeterShaggyNoble added awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed changes requested labels Jun 9, 2020
@ericcornelissen
Copy link
Contributor

#77216F (Light aubergine) comes from the colour pallete and is the closest to what's in the SVG, that's why I suggested it 🙂

@PeterShaggyNoble PeterShaggyNoble removed the awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed label Jun 9, 2020
@PeterShaggyNoble
Copy link
Member

Ah, hadn't seen that. In that case, I agree with the choice of colour.

@gizmecano
Copy link
Contributor Author

(...) it just needs to be centred on the canvas.

I don't quite understand what @PeterShaggyNoble means...

The path seems absolutely centered in my editor (Inkscape) and I don't really see what I can change here... 🤔

fit-canvas

Can you explain me what seems erroneous on your own side?

@PeterShaggyNoble

This comment has been minimized.

@PeterShaggyNoble
Copy link
Member

PeterShaggyNoble commented Jun 9, 2020

D'oh! I hadn't spotted that the x & y coordinates of the viewBox in the original weren't set to 0, accounting for the offset I thought I was seeing. Sorry 'bout that.

Looks perfect to me now - thanks again for your efforts on this one, @gizmecano 👍

@PeterShaggyNoble PeterShaggyNoble merged commit faaf49d into simple-icons:develop Jun 9, 2020
@gizmecano
Copy link
Contributor Author

Do not hesitate to tell me if you see such an error: I have to say I'm not necessarily experienced enough for checking all required elements.

@gizmecano gizmecano deleted the input/canonical branch June 9, 2020 15:45
@gizmecano gizmecano mentioned this pull request Jun 9, 2020
ericcornelissen added a commit that referenced this pull request Jun 14, 2020
# New Icons

- Canonical (#3184)
- Der Spiegel (#3168)
- DHL (#3048)
- Land Rover (#3195)
- Openlayers (#3165)
- Prime (#3010)
- Quasar (#3144)

# Updated Icons

- Apache ECharts (#3166)
- Apache Pulsar (#3181)
- Fur Affinity (#2979)
- Jupyter (#3170)
@gizmecano gizmecano mentioned this pull request Aug 17, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new icon Issues or pull requests for adding a new icon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants