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

box-shadow and text-shadow support #149

Closed
wants to merge 16 commits into from
Closed

box-shadow and text-shadow support #149

wants to merge 16 commits into from

Conversation

liZe
Copy link
Member

@liZe liZe commented Dec 27, 2013

Fix #13.

@Le-Stagiaire
Copy link
Member

Awesome work !

@aaronpeterson
Copy link

Can this still be merged?

@liZe
Copy link
Member Author

liZe commented Oct 2, 2014

It works quite well and probably can be merged. The main problem is that the changes rely on PIL/pillow. @SimonSapin what do you think about that?

@SimonSapin
Copy link
Member

Requiring PIL makes me very sad, but I don’t have an alternative to propose right now.

@liZe
Copy link
Member Author

liZe commented Oct 2, 2014

Of course, another big problem of this implementation is that it relies on bitmaps for the shadows, even in PDFs. But as Simon, this bad solution is the best solution I can think of.

The real solution is to have this in cairo, but that will probably never happen.
http://lists.freedesktop.org/archives/cairo/2012-June/023287.html

IMO, we can merge this PR because the added code is small, simple, and doesn't change anything in the architecture. We must open another bug to remember that pixmaps in PDFs are bad and that this implementation is slooooow as hell.

Oh, and I'd really like someone to remove this awful 4/3 ratio before merging.

thickness, enable_hinting)

for x, y, blur, color in reversed(textbox.style.text_shadow):
# TODO: fix this 4/3 ratio
Copy link
Member Author

Choose a reason for hiding this comment

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

Please, fix me before merging!

@liZe
Copy link
Member Author

liZe commented Jul 28, 2016

I close this PR as it won't be merged. Depending on the PIL and using raster blurs are an awful solution.

@liZe liZe closed this Jul 28, 2016
quis added a commit to alphagov/notifications-utils that referenced this pull request Dec 9, 2016
This commit highlights placeholders in letter previews the same way
that we do for emails and text messages. The implementation is slightly
different because WeasyPrint doesn’t support box-shadow (see
Kozea/WeasyPrint#149 (comment) for
reasons).

Luckily WeasyPrint _does_ support CSS3 multiple backgrounds, so we can
mask the curved edges with SVG background images instead. This wouldn’t
be a great solution on the web because it you might get a flash of
unstyled placeholder as the images loaded. But because the PDF is
rendered completely before it’s served it shouldn’t be a problem.
quis added a commit to alphagov/notifications-utils that referenced this pull request Dec 13, 2016
This commit highlights placeholders in letter previews the same way
that we do for emails and text messages. The implementation is slightly
different because WeasyPrint doesn’t support box-shadow (see
Kozea/WeasyPrint#149 (comment) for
reasons).

Luckily WeasyPrint _does_ support CSS3 multiple backgrounds, so we can
mask the curved edges with SVG background images instead. This wouldn’t
be a great solution on the web because it you might get a flash of
unstyled placeholder as the images loaded. But because the PDF is
rendered completely before it’s served it shouldn’t be a problem.
quis added a commit to alphagov/notifications-utils that referenced this pull request Dec 14, 2016
This commit highlights placeholders in letter previews the same way
that we do for emails and text messages. The implementation is slightly
different because WeasyPrint doesn’t support box-shadow (see
Kozea/WeasyPrint#149 (comment) for
reasons).

Luckily WeasyPrint _does_ support CSS3 multiple backgrounds, so we can
mask the curved edges with SVG background images instead. This wouldn’t
be a great solution on the web because it you might get a flash of
unstyled placeholder as the images loaded. But because the PDF is
rendered completely before it’s served it shouldn’t be a problem.
quis added a commit to alphagov/notifications-utils that referenced this pull request Dec 14, 2016
This commit highlights placeholders in letter previews the same way
that we do for emails and text messages. The implementation is slightly
different because WeasyPrint doesn’t support box-shadow (see
Kozea/WeasyPrint#149 (comment) for
reasons).

Luckily WeasyPrint _does_ support CSS3 multiple backgrounds, so we can
mask the curved edges with SVG background images instead. This wouldn’t
be a great solution on the web because it you might get a flash of
unstyled placeholder as the images loaded. But because the PDF is
rendered completely before it’s served it shouldn’t be a problem.
@Afoucaul
Copy link

Afoucaul commented Apr 26, 2019

Hi, is there any plan to support box-shadow one day? Is there any work in progress other than this PR? I'd be glad to help, but I'm a bit lost in the code, and I have troubles getting the test to run from this PR. If you updated your branch, that would help me start working on this feature.

@liZe liZe deleted the shadow branch July 13, 2019 11:41
@liZe liZe mentioned this pull request Nov 6, 2019
@liZe
Copy link
Member Author

liZe commented Nov 6, 2019

Hi, is there any plan to support box-shadow one day? Is there any work in progress other than this PR? I'd be glad to help, but I'm a bit lost in the code, and I have troubles getting the test to run from this PR. If you updated your branch, that would help me start working on this feature.

This pull request generates a raster image included in the generated PDF, that's an ugly solution. Unless there's a clean way to include blurs in PDF files, and unless this feature is included in Cairo, I don't see how we could include blurs in a clean way.

@hlandau
Copy link

hlandau commented Sep 20, 2022

Worth noting that box shadow can be used without blurs. So I guess it would be possible to support a subset of box-shadow where blur isn't used.

@liZe
Copy link
Member Author

liZe commented Sep 22, 2022

Worth noting that box shadow can be used without blurs. So I guess it would be possible to support a subset of box-shadow where blur isn't used.

That’s right, and that would be useful in some cases. We can continue to discuss on #13 and see if some people are interested in providing and/or using this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature that should be supported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS box-shadow and text-shadow
6 participants