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

Suggestion: External control of wrapping criteria #32

Closed
StylingAndroid opened this issue May 14, 2016 · 6 comments

Comments

@StylingAndroid
Copy link

commented May 14, 2016

While it would certainly diverge from the CSS flexbox model upon which this library is based, there are some inherent inefficiencies for Android because of differences in cost in nesting ViewGroups in Android compared to nesting <div> elements in HTML.

For tabular data, the following is simple and efficient in HTML:

<div class="grid">
  <div class="grid__row">
    <div class="grid__item">1</div>
    <div class="grid__item">2</div>
  </div>
  <div class="grid__row">
    <div class="grid__item">1</div>
    <div class="grid__item">2</div>
    <div class="grid__item">3</div>
  </div>
  <div class="grid__row">
    <div class="grid__item">1</div>
    <div class="grid__item">2</div>
    <div class="grid__item">3</div>
    <div class="grid__item">4</div>
  </div>
</div>

And CSS flexbox can easily be applied to this. However for Android This would need to be done with a parent ViewGroup representing grid, containing an number for child grid_row ViewGroups, each containing a collection of Views representing the individual the individual cells. Such a hierarchy is somewhat inefficient both in terms of layout inflation, and during measurement and layout.

A couple of simple additions to FlexboxLayout could make it easy to flatten such layouts by removing the individual rows - FlexboxLayout manages a collection of FlexLine objects which get calculated during measurement - and this is much more efficient that having child ViewGroups representing individual rows. If it were possible to have some optional attributes which could control when we should wrap to a new FlexLine then the usefulness of FlexboxLayout would increase dramatically, I think. Specifically we could have a flexLineItems integer which would cause a wrap whenever the current FlexLine contains the specified number of items. In addition a LayoutParams.wrapAfter boolean (default to false) could be used to force a wrap after the current child View.

As I said, I appreciate that this is a deviation from the CSS flexbox model, but feel that it could be a really useful addition on Android.

@thagikura

This comment has been minimized.

Copy link
Member

commented May 15, 2016

Hi,

Thanks for your suggestion.
But I think having a explicit flag to indicate from which flex item the wrap should happen is not a way to go.

Because you don't have to nest ViewGroups to do so in the grid example above.
If you want to build a grid with the FlexboxLayout you can put the grid cells as direct children of the FlexboxLayout.

E.g. If you want make the first row having two cells, the second row having three cells and the third row having four cells, you can write a XML like below (omitting the not relevant attributes).

<com.google.android.flexbox.Flexbox
  app:flexWrap="wrap">

    <View app:layout_flexBasisPercent="50%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="50%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="30%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="30%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="30%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="25%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="25%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="25%"
          app:layout_flexGrow="1">
    <View app:layout_flexBasisPercent="25%"
          app:layout_flexGrow="1">

</com.google.android.flexbox.Flexbox>

I think the layout above gives you the same result as you HTML as you pasted.

Instead if you want to give each flex item a fixed initial width and want to make the FlexboxLayout grow (have a new flex line) when a flex item doesn't fit in the current flex line, you can also do so like the example in the wiki

Having that flexibility is one of the big advantages of the Flexbox and I think adding a explicit flag to indicate from which flex item the wrap should happen loses that flexibility.

How do you think?

@StylingAndroid

This comment has been minimized.

Copy link
Author

commented May 15, 2016

But that only works if you manually specify the percentages and can already be achieve efficiently on Android using PercentLayout. Having the ability to control when wraps occur would make FlexboxLayout totally unique for Android, and would open up many more use-cases which only FlexboxLayout would provide an efficient solution.

@thagikura

This comment has been minimized.

Copy link
Member

commented May 15, 2016

Ok, I understand controlling when wrap occurs is equivalent to having fixed number of items in each grid row.
So I think it can be achieved by having different span counts on each row in a grid using GridLayoutManager like this.
Does that meet your requirement?

@StylingAndroid

This comment has been minimized.

Copy link
Author

commented May 15, 2016

That would work, but would further compilicate an API which already isn't that intuitive for Android devs (alignItems vs. alignContent). That's the primary reason I'm suggesting what I am - increased control over internal behaviour via simple, understandable API additions.

However, any additions to the functionality which would facilitate flattened layouts for tabular data would be greatly welcomed by me ;-)

@thagikura

This comment has been minimized.

Copy link
Member

commented Jun 2, 2016

Sorry for the long delay.
During the course of replacing the existing LinearLayout with FlexboxLayout ( #59 ), I realized that attribute is useful in many situations.

I'm going to add this request to the queue of the upcoming implementation plan.
Probably in the form of layout_wrapAfter or layout_wrapBefore

Thanks.

@thagikura

This comment has been minimized.

Copy link
Member

commented Jun 3, 2016

Fixed by #60
I'm going to include it in the next release.

@thagikura thagikura closed this Jun 3, 2016

@thagikura thagikura added this to the 0.2.1 milestone Jun 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.