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

LibWeb/CSS: If possible, simplify min() and max() calculations #1800

Conversation

Simek
Copy link

@Simek Simek commented Oct 14, 2024

Why

According to the spec, the min() and max() calculation can be simplified, if there is only one child:

Should fix few WPT tests, for example:

How

Return the sole child string value directly in to_string calls on MinCalculationNode and MaxCalculationNode nodes.

Preview

Screenshot 2024-10-14 at 23 14 19

@Simek Simek requested a review from AtkinsSJ as a code owner October 14, 2024 21:19
@awesomekling
Copy link
Member

If I understand correctly, this should happen through what the spec calls simplification, not by hacking individual nodes to serialize differently. @AtkinsSJ confirm?

@stelar7
Copy link
Contributor

stelar7 commented Oct 15, 2024

This should happen as part of parse_a_calculation

@AtkinsSJ
Copy link
Member

If I understand correctly, this should happen through what the spec calls simplification, not by hacking individual nodes to serialize differently. @AtkinsSJ confirm?

Indeed. I think this is the wrong approach to passing the test.

This should happen as part of parse_a_calculation

Also yep, it's the final step there.

Simplification is top of my list as soon as I get nesting done.

@Simek
Copy link
Author

Simek commented Oct 16, 2024

Thanks for the feedback everyone! 👍

Just want to try what's the project entry level by those PRs, and how experimental is the dev phase. Definitely need to spent more time reading the code than for an hour, to get grasp of everything what is happening, hence going to close this contribution for now. Maybe I will follow up on this in the future, if I find more time to dive deeper into the project.

@Simek Simek closed this Oct 16, 2024
@Simek Simek deleted the @simek/ladybird-css-min-max-simplify branch October 16, 2024 08:35
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.

4 participants