Skip to content

Grid: Fix sizing issues in complex cases#355

Merged
nicoburns merged 2 commits intoDioxusLabs:mainfrom
nicoburns:grid/fix-sizing-issues
Feb 7, 2023
Merged

Grid: Fix sizing issues in complex cases#355
nicoburns merged 2 commits intoDioxusLabs:mainfrom
nicoburns:grid/fix-sizing-issues

Conversation

@nicoburns
Copy link
Copy Markdown
Collaborator

Objective

Correctness fixes for the CSS Grid algorithm

Changes made

  • Don't constrain grid item's avaiable space based on a grid container's definite available space
  • Pass estimates from §11.1. Grid Sizing Algorithm as parent_size/available_space rather than known_dimensions
  • Apply stretch sizing to compute estimated known_dimensions for grid items when sizing tracks
  • Prioritise align/justify self over parent's align/justify items

Note: This PR includes the changes from #354 so #354 should be merged before this one.

@nicoburns nicoburns added bug Something isn't working blocked Cannot be advanced until something else changes labels Feb 7, 2023
@nicoburns nicoburns added this to the 0.3 "CSS Grid" milestone Feb 7, 2023
@nicoburns nicoburns mentioned this pull request Feb 7, 2023
87 tasks
@alice-i-cecile
Copy link
Copy Markdown
Collaborator

Can you rebase so we can review more easily?

- Don't constrain grid item's avaiable space based on a grid container's definite available space
- Pass estimates from §11.1. Grid Sizing Algorithm as available space rather than known_dimensions
- Apply stretch sizing to compute known_dimensions when sizing tracks
- Prioritise align/justify self over parent's align/justify items
@nicoburns nicoburns force-pushed the grid/fix-sizing-issues branch from d546fe1 to cb7f68a Compare February 7, 2023 18:12
@nicoburns nicoburns removed the blocked Cannot be advanced until something else changes label Feb 7, 2023
@nicoburns
Copy link
Copy Markdown
Collaborator Author

Can you rebase so we can review more easily?

Done!

Copy link
Copy Markdown
Collaborator

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable set of changes, and good tests like always.

@nicoburns nicoburns merged commit 7672268 into DioxusLabs:main Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants