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

Miscellaneous corrections to Pack #1958

Merged
merged 9 commits into from
Jun 5, 2023
Merged

Conversation

freakboy3742
Copy link
Member

This PR addresses some issues with the Pack layout algorithm that were found in the process of developing #1956.

  • Converts the Pack tests to pytest
  • Adds tests to get 100% branch coverage for Pack, including CSS rendering
  • Allows a node to have an explicit width/height of 0.
  • Alters the default width/height of a node to be NONE, rather than 0. This matches CSS, and is needed to ensure that nodes have a "floating" width/height unless a size is explicitly provided.
  • Ensures that if a node has an intrinsic size of 0, it is honoured, rather than being ignored.
  • Replaces the special handling of root as an "expanding" node with a more generic handling that ensures that column boxes won't have a their width collapsed, and row boxes won't have their height collapsed. This has a signifiant impact on alignment tests (in particular, the row/column alignment and rtl tests); a row box in a row box will now preserve height on the inner row box, and as a result, centre alignment on the inner row box will introduce a larger vertical offset - children of the inner row box will be vertical centered on the window, not just the content of the inner row box. A similar case exists for content in column boxes.

The test cases for the tutorials + beeliza are unchanged.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Copy link
Member

@mhsmith mhsmith left a comment

Choose a reason for hiding this comment

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

I still can't claim to have a strong understanding of the layout algorithm, but everything looks reasonable.

@freakboy3742 freakboy3742 merged commit 8c5e97a into beeware:main Jun 5, 2023
@freakboy3742 freakboy3742 deleted the pack-fixes branch June 5, 2023 22:38
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