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

added new TPLINK logo with brand name #8613

Merged
merged 4 commits into from Apr 25, 2018

Conversation

Projects
None yet
4 participants
@czilian
Copy link
Contributor

czilian commented Apr 25, 2018

added "TP Link" underneath previous SVG graphic. (Logo itself is very little self-explanatory without)

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 25, 2018

Icons aren't meant to be self explanatory. They are meant for quick identification.

So unless this is similar to another icon I don't think this is a good change. I'm not a fan of text in the icons.

However, I do think you should add a logo image with the tplink text to the right. https://docs.librenms.org/Developing/os/Initial-Detection/#icon-and-logo

Also, the font on your tp-link is wrong.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 25, 2018

I actually found a nice logo from wikipedia so I just added that for you. I think we should revert the icon. Does that sound ok to you?

@czilian

This comment has been minimized.

Copy link
Contributor Author

czilian commented Apr 25, 2018

Yes it is. Use this one then. Thx

laf added some commits Apr 25, 2018

@laf

laf approved these changes Apr 25, 2018

Copy link
Member

laf left a comment

LGTM.

@murrant you added an svg with width/height :p

@laf

laf approved these changes Apr 25, 2018

Copy link
Member

laf left a comment

LGTM.

@murrant you added an svg with width/height :p

@scrutinizer-notifier

This comment has been minimized.

Copy link

scrutinizer-notifier commented Apr 25, 2018

The inspection completed: No new issues

@laf laf merged commit 492a973 into librenms:master Apr 25, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@murrant

This comment has been minimized.

Copy link
Member

murrant commented Apr 25, 2018

Sorry, thought I had those options checked for svgo

@czilian czilian deleted the czilian:newTPLINKlogo branch Apr 25, 2018

TheMysteriousX added a commit to TheMysteriousX/librenms that referenced this pull request May 20, 2018

webui: Added new TPLINK logo with brand name (librenms#8613)
* added new TPLINK logo with brand name

* Create tplink.svg logo

* Update tplink.svg

* Update tplink.svg

@lock lock bot locked as resolved and limited conversation to collaborators Jun 25, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.