Closed dandaka closed 8 years ago
LGTM.
If you could do an 'OLE' logo for Open Lighting Embedded that would be great. We don't have one:
Sorry @dandaka can these be the same versions as used in the web UI. It looks like the web UI is W: 51px | H: 21px, but what you've uploaded is W: 54px | H: 22px . Or will the wider logo fit the UI?
We have an OLE logo @nomis52 it just looks rather rough: https://github.com/OpenLightingProject/logos/pull/6/files
So, the backstory. In the original web UI, the OLA logo is placed right up against the edge of the page, and I wanted to create a drop-in replacement for it without having to touch CSS and layout. Hence, the empty pixels exist to give it a little spacing. This matched roughly what the old file had in it.
I agree it’s nice to not have empty pixels, and they are unnecessary for new work.
However, the solution should have been to simply trim the empty pixels off, not to scale up the logo to fill them. I paid attention to making the logo align to pixel boundaries so it is clear at the small sizes, and not blurry. Hence the name of the directory "web fitted".
As seen in the attached screenshot, the new versions (top) disregard this, and have blurry edges everywhere.
Yeah, I know this is a bit pedantic, but I would like to fix this up, and will submit a pull request soon.
Ah yes, looking the logos now I can see the difference!
Thanks for the comments @DouglasHeriot . I certainly appreciate the attention to detail.
So to muddy the waters further, as per my comments above and the commit here: https://github.com/dandaka/ola/commit/020d0914367db122321eca3e33e5428150aca470
Would I be thinking the OLA repo is currently correct for the new UI, which doesn't need the padding built in, because the image height has just shrunk, presumably by the removal of the blank row? It's the logo repo that's wrong.
I think ideally we want old UI 22px and 22px@2x (mobile) and 48px and 48px (normal) and new UI (no blank pixels) 22px and 22px@2x images. The RDM test server also uses the large image, but I don't know if it needs the blank pixels or not.
@DouglasHeriot Your version is clearly better, but requires manual tuning in all cases. I would be glad, if you fix this.
As requested in #7