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

Support aspect-ratio for Leaf nodes #313

Closed
adjabaev opened this issue Jan 4, 2023 · 1 comment · Fixed by #317
Closed

Support aspect-ratio for Leaf nodes #313

adjabaev opened this issue Jan 4, 2023 · 1 comment · Fixed by #317
Labels
enhancement New feature or request

Comments

@adjabaev
Copy link
Contributor

adjabaev commented Jan 4, 2023

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

The css aspect ratio property isn't supposed for leaf nodes.

What solution would you like?

The support for that property out of the box
Also it would be nice to test if this property works well for grid/ flex nodes (I didn't find any test testing the property)

What alternative(s) have you considered?

  • Entering cross axis size manually
  • Implementing this by myself, unfortunately the knowDimensions.width that passes into Leaf's compute function is equal to 100f32 (more context in the gist below) which is the parent's width (does this make sense?). Getting cross axis would be easy if that parameter was equal to Option::None. Also this is the first rust project i'm working with, I don't feel very confident in what I am doing yet.

Additional context

I've done some researches to ensure this wasn't supported
Here is an adaptation of the test script (js) to take aspectRatio into account aswell as a simple test that shows the issue (rs & html files)
https://gist.github.com/adjabaev/4dfd194ddee6becab49c9dbdbd191baa

@adjabaev adjabaev added the enhancement New feature or request label Jan 4, 2023
@nicoburns
Copy link
Collaborator

Yeah, we definitely need to make this work. It was on my list to make this work for grid, but you're right that is more generally poorly supported (I hadn't realised that there were no tests for aspect ratio at all!). I'll take a look and see what is required. If you want to help, then more test cases / examples of HTML snippets you think ought to work would be helpful. Off the top of my head, I think we will need to support:

  • Both width and height
  • Also min and max width/height
  • Children that are within Flex containers and Grid Containers
  • Children that are Leaf nodes, Flex nodes and Grid nodes.

If you do have time to come up with more test cases, I would recommend posting them in the following format:

// aspect_ratio_leaf_node
<div id="test-root" style="height: 100px; width: 100px;">
  <div style="height: 50%; aspect-ratio: 1;"></div>
</div>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants