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

SVG images #2995

Merged
merged 3 commits into from Dec 21, 2018
Merged

SVG images #2995

merged 3 commits into from Dec 21, 2018

Conversation

elia
Copy link
Member

@elia elia commented Dec 7, 2018

Use SVG images in a couple of places to make everything look better.

Before:

screenshot 2018-12-21 at 17 34 03

screenshot 2018-12-21 at 17 33 53

After:

screenshot 2018-12-19 at 14 54 29

screenshot 2018-12-19 at 14 54 17

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks @elia I think we should keep the png version of the logo since that preference has probably already been defined in each store and this will make them ask for a not existing image.

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

I'm okay with changing the README, but I'm hesitant about changing the admin UI from png.

kennyadsl
kennyadsl previously approved these changes Dec 8, 2018
@kennyadsl
Copy link
Member

@elia I think we need some padding since the svg is taking all the space available:

schermata 2018-12-08 alle 18 35 56

@kennyadsl kennyadsl dismissed their stale review December 8, 2018 17:37

We still need some spacing adjustment

@elia
Copy link
Member Author

elia commented Dec 21, 2018

I've fixed the internal SVG size and padding and updated the description with before/after screenshots.

Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

The screenshots look good! 👍

@jacobherrington jacobherrington merged commit 7b6b50f into solidusio:master Dec 21, 2018
@elia elia deleted the elia/svg-images branch December 21, 2018 17:03
Sinetheta added a commit to Sinetheta/solidus_print_invoice that referenced this pull request Jan 13, 2019
The Solidus admin area was recently changed to use an svg logo
solidusio/solidus#2995
but that is not supported by Prawn so it broke the pdf embed here.

The old png logo is no longer used in Solidus so it's only a matter of
time before someone removes it. As such I have pulled it in for use
in the pdf header, but kept the same name because I figured it less
likely that the png logo ever be updated again in Solidus than maybe
someone customized their Solidus install by overwriting the image
instead of changing the config.

This has the unfortunate affect of no longer automatically following
Solidus admin logo customizations, but I don't think it would be worth
doing fancy svg detection at that point. We have our own config, folks
can use that if needed.
aitbw added a commit to aitbw/solidus_print_invoice that referenced this pull request Jan 15, 2019
solidusio/solidus#2995 replaced Solidus' .png logo with the SVG
version and Prawn doesn't know how to render this kind of files
Sinetheta added a commit to Sinetheta/solidus_print_invoice that referenced this pull request Jan 16, 2019
The Solidus admin area was recently changed to use an svg logo
solidusio/solidus#2995
but that is not supported by Prawn so it broke the pdf embed here.

The old png logo is no longer used in Solidus so it's only a matter of
time before someone removes it. As such I have pulled it in for use
in the pdf header, but kept the same name because I figured it less
likely that the png logo ever be updated again in Solidus than maybe
someone customized their Solidus install by overwriting the image
instead of changing the config.

This has the unfortunate affect of no longer automatically following
Solidus admin logo customizations, but I don't think it would be worth
doing fancy svg detection at that point. We have our own config, folks
can use that if needed.
aitbw added a commit to aitbw/solidus_print_invoice that referenced this pull request Feb 7, 2019
solidusio/solidus#2995 replaced Solidus' .png logo with a .svg version,
a format that Prawn does not support. This patch re-introduces the former
logo and uses it if no custom configuration is provided.
aitbw added a commit to aitbw/solidus_print_invoice that referenced this pull request Feb 7, 2019
solidusio/solidus#2995 replaced Solidus' .png logo with a .svg version,
a format that Prawn does not support. This patch re-introduces the former
logo and uses it if no custom configuration is provided.
aitbw added a commit to aitbw/solidus_print_invoice that referenced this pull request Feb 11, 2019
solidusio/solidus#2995 replaced Solidus' .png logo with a .svg version,
a format that Prawn does not support. This patch re-introduces the former
logo and uses it if no custom configuration is provided.
aitbw added a commit to aitbw/solidus_print_invoice that referenced this pull request Feb 11, 2019
solidusio/solidus#2995 replaced Solidus' .png logo with a .svg version,
a format that Prawn does not support. This patch re-introduces the former
logo and uses it if no custom configuration is provided.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants