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

Add tests for invalid style inputs #553

Open
nicoburns opened this issue Oct 5, 2023 · 3 comments
Open

Add tests for invalid style inputs #553

nicoburns opened this issue Oct 5, 2023 · 3 comments
Labels
testing Additional tests or improvements to the testing infrastructure

Comments

@nicoburns
Copy link
Collaborator

What problem does this solve or what need does it fill?

Some style properties accept values that aren't actually valid for that property. The most notable cases being properties that accept f32 but where only non-negative finite values are valid.

What solution would you like?

  • Create tests for these cases.
  • Ensure that Taffy's algorithms are correctly discarding these styles and using the default value for the style property in their place.

What alternative(s) have you considered?

  • Create restricted types like NonNegativeF32. However, this seems like it might be a pain for consumers of the API who will have to push values through a conversion step every time they want to set a style.

Additional context

We can't easily cover these cases with gentests as the browser will discard the styles before they even get to the

@nicoburns nicoburns added the testing Additional tests or improvements to the testing infrastructure label Oct 5, 2023
@alice-i-cecile
Copy link
Collaborator

I like this! Unfortunately the NonNegativef32 types and friends are a pain to work with for users; I think we should avoid them in our API.

That said, I'd be fully on-board for adding a ton of debug asserts to check for invalid data like this as well. Users can disable them for their code by building taffy itself in release mode.

@nicoburns
Copy link
Collaborator Author

I like this! Unfortunately the NonNegativef32 types and friends are a pain to work with for users; I think we should avoid them in our API.

Yeah, agreed.

That said, I'd be fully on-board for adding a ton of debug asserts to check for invalid data like this as well. Users can disable them for their code by building taffy itself in release mode.

I think my ideal is something like .lint() method on Style that returns something like a Vec<StyleWarning> describing any issues, and could be used either for logging warnings, creating hard errors, or implementing something akin to browser dev tools. And perhaps something similar for a whole tree?

@alice-i-cecile
Copy link
Collaborator

Okay, I'm on board with that :) Definitely want something for the whole tree too; libraries will want to be able to expose this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Additional tests or improvements to the testing infrastructure
Projects
None yet
Development

No branches or pull requests

2 participants