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

GitHub issue #8 and couple of minor changes #9

Merged
merged 3 commits into from Dec 15, 2017

Conversation

Projects
None yet
2 participants
@ohhai
Copy link
Contributor

ohhai commented Dec 14, 2017

No description provided.

@ckuhl

This comment has been minimized.

Copy link
Owner

ckuhl commented Dec 14, 2017

Maybe it's just my browser, but image_blocked.svg and image.svg look the same to me - is it different on your end? Aside from that the pull request looks good 👍

@ohhai

This comment has been minimized.

Copy link
Contributor Author

ohhai commented Dec 14, 2017

Yes, I saw it in other viewers--related to "X" is a font char, I think. You could open it with any editor, I used Inkscape (or convert via imagemagick), X should be present for these cases.

@ckuhl

This comment has been minimized.

Copy link
Owner

ckuhl commented Dec 14, 2017

I see now. However testing it on Ubuntu on Windows 10, it mangles the images when it converts them to png:

image_blocked-256

@ohhai

This comment has been minimized.

Copy link
Contributor Author

ohhai commented Dec 15, 2017

Wow.
Does it happen with original Ionicons image?
As for Ubuntu on Windows — you mean virtual machine (VirtualBox?)

@ckuhl

This comment has been minimized.

Copy link
Owner

ckuhl commented Dec 15, 2017

The original icon is fine in all of it's sizes, and all of the blocked images are similarly distorted

image

Running convert --version I see that I am running ImageMagick 6.8.9-9. What version are you running?

About Ubuntu on Windows, it's a translation layer for linux syscalls(?)

@ohhai

This comment has been minimized.

Copy link
Contributor Author

ohhai commented Dec 15, 2017

Oh. My hope was that it's not that case (official Ubuntu from Microsoft).

I use ImageMagick 6.9.9.22 from Fedora repos.
Can verify on Ubuntu 16.04 LTS (this version is pointed by the link to MS store), but does it have a sense?
I mean, if you use this ubuntu-on-windows as main development environment.

Tried a common alternative tool advised in internets: rsvg.
But it just ignores 'X' as in previous cases.

Workaround might be to convert X to a vector object, not text.
It might remove all the issues and it's not hard to re-create it as text if necessary.
I'm attaching such a SVG (gzipped, github does want it plain).
image_blocked_notext.svg.gz

@ckuhl

This comment has been minimized.

Copy link
Owner

ckuhl commented Dec 15, 2017

That new version does the trick! Commit it in and I'll merge in this request 👍

ohhai added some commits Dec 14, 2017

[icons] generate PNG icons from outlined SVG source
GitHub issue #8: "[wishlist] please use more contrast button icon"

Current icons are black and hardly visible on dark FF themes.
Use modified SVG source with light outline for better visibility.

Remove PNG files from repo and generate them in makefile.

Add 16x16 and 24x24 png size as well.

SVG taken from Ionicons iconset (http://ionicons.com/)
and are used here under the MIT license: http://opensource.org/licenses/MIT
[makefile] add -fv parameters for rm
- don't produce error if some files are absent
    and it seems better than prefixing make command with '-'
- print actually removed files
- it's not interactive here anyway

@ohhai ohhai force-pushed the ohhai:master branch from 959fddc to 0fa7f1f Dec 15, 2017

@ohhai

This comment has been minimized.

Copy link
Contributor Author

ohhai commented Dec 15, 2017

Updated.

@ckuhl

ckuhl approved these changes Dec 15, 2017

@ckuhl ckuhl merged commit a67112a into ckuhl:master Dec 15, 2017

@ckuhl

This comment has been minimized.

Copy link
Owner

ckuhl commented Dec 15, 2017

It's been merged in, packaged, and uploaded to AMO! If you update your addons now you should see your new icon 😄

@ohhai

This comment has been minimized.

Copy link
Contributor Author

ohhai commented Dec 15, 2017

Updated, see it, thanks a lot!

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