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 Align::Fill variant #1044

Merged
merged 5 commits into from
Sep 20, 2021
Merged

Conversation

tarkah
Copy link
Member

@tarkah tarkah commented Sep 14, 2021

Adds a new Align::Fill variant that works similar to align-items: stretch;.

Currently, it's impossible to have all child elements of a row or column "fill" the remaining space based on the largest sized child. This makes using widgets like Rule impossible when nested inside a row or column that has no fixed size.

Here's an example of how it's possible to use Rule in the pokedex example, where it's height now stretches to fill the dynamic height of the row it's in. Without Align::Fill, the Rule expands to fill the entire viewport.

Before:

Screenshot from 2021-09-14 10:00:27

After:

Screenshot from 2021-09-14 10:01:56

Closes #746

native/src/layout/flex.rs Outdated Show resolved Hide resolved
Copy link
Member

@hecrj hecrj left a comment

Choose a reason for hiding this comment

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

Looks great! 🥳

It's really nice that we can extend the layout engine so easily with just an additional variant! 🎉

I left a couple of suggestions. Let me know what you think!

native/src/layout/flex.rs Outdated Show resolved Hide resolved
native/src/layout/flex.rs Outdated Show resolved Hide resolved
native/src/layout/flex.rs Outdated Show resolved Hide resolved
@hecrj hecrj added the feature New feature or request label Sep 15, 2021
@hecrj hecrj added this to the 0.4.0 milestone Sep 15, 2021
@tarkah
Copy link
Member Author

tarkah commented Sep 15, 2021

Thanks, @hecrj! Agreed, a surprisingly easy change for a big layout improvement.

One last question before we merge... I'm having a harder time reasoning about the potential impact of this change on Container, since it accepts Align in align_x and align_y, but it only has "one" child. Do we need to handle anything special in layout of Container?

@tarkah tarkah requested a review from hecrj September 15, 2021 18:24
@hecrj
Copy link
Member

hecrj commented Sep 20, 2021

@tarkah I think the changes in Node::align should take care of it. We could also create a new enum exclusively for cross alignment.

@hecrj
Copy link
Member

hecrj commented Sep 20, 2021

I have rebased and refactored the alignment types by moving them into a new alignment module.

Also, I have changed the Container API to leverage alignment::Horizontal and alignment::Vertical directly, which have no Fill variant.

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

Successfully merging this pull request may close these issues.

How can I flex a collection of elements to be the same width/fill remaining space?
2 participants