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

fix #1007 Make 'inherit' value work correctly for shorthand properties #1522

Closed
wants to merge 1 commit into from

Conversation

ckepper
Copy link
Contributor

@ckepper ckepper commented Dec 13, 2021

  • skip shorthand expansion for 'inherit' tokens, hereby applying parent styles

This is my first contribution to WeasyPrint - I am not sure if this actually fixes the issue or just makes the error message go away.

…erties

* skip shorthand expansion for 'inherit' tokens, hereby applying parent styles
@liZe
Copy link
Member

liZe commented Dec 14, 2021

Hello!

This is my first contribution to WeasyPrint

Thanks a lot for this PR!

I am not sure if this actually fixes the issue or just makes the error message go away.

It makes the error message go away, but unfortunately it doesn’t fix the problem :/.

But there’s actually a decorator that already does what we want: @generic_expander is able to do what we want, and we have to use it everywhere. Unfortunately, there are lots of little details to take care about, so that’s not an easy fix for a first contribution. I can try to write a nice PR so that you can learn more about the validation system, and maybe open another PR in the future!

@liZe liZe closed this Dec 14, 2021
@ckepper
Copy link
Contributor Author

ckepper commented Dec 14, 2021

Understood. Thank you for responding so quickly. I will gladly analyze your PR when it's ready to learn more about how WeasyPrint works.

Are there other issues you would recommend to work on? I checked the good first issue list but nothing else jumped at me right away.

@liZe
Copy link
Member

liZe commented Dec 14, 2021

The easiest issue is probably to add the support of inherit to other properties. flex-flow, page-break-inside and word-wrap are probably the easiest ones.

Just like I did for text-decoration, you can add the @generic_expander decorator and replace the yielded values (often strings or numbers) by a list of real tokens. You can then test with a real document, and add a test if it works.

If it’s too complicated or too abstract (diving into WeasyPrint is a bit hard, that’s totally normal), I can give more information and/or add an example with more details in #1524.

#1512 and #1376 are also good issues to start, quite interesting with a small amount of code to write and visible results. They probably require some details and hints, but don’t hesitate to add a comment in these tickets if the topic is interesting for you!

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.

2 participants