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

lost-align issue #233

Closed
eybarta opened this Issue Jan 13, 2016 · 8 comments

Comments

@eybarta

eybarta commented Jan 13, 2016

Thx for all your work, very refreshing using this grid and thx Peter for maintaining!

I'm not sure if I'm doing something wrong.. but using lost-align on a parent seems to give the justify-content and align-items properties to the children instead of the parent resulting in incorrect behavior.. I looked @ lost/lib/lost-align.js and it seems the conditional styles are always given to ' > * ',
am I misunderstanding something or is this a bug?

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Jan 13, 2016

Owner

@eybarta I might be misunderstanding this but it looks to be this works as expected. lost-align is intended to align the child elements. See here: lost-align

Align nested elements. Apply this to a parent container.

Please let me know if this I'm not understanding this correctly. Maybe some of the code you're working with? Thanks!

Owner

peterramsing commented Jan 13, 2016

@eybarta I might be misunderstanding this but it looks to be this works as expected. lost-align is intended to align the child elements. See here: lost-align

Align nested elements. Apply this to a parent container.

Please let me know if this I'm not understanding this correctly. Maybe some of the code you're working with? Thanks!

@corysimmons

This comment has been minimized.

Show comment
Hide comment
@corysimmons

corysimmons Jan 13, 2016

Contributor

Crap, I think @eybarta is right. justify-content and align-items should be applied to the parent container instead of > *.

lost-align is broken when used with global flexbox or the flex param.

Contributor

corysimmons commented Jan 13, 2016

Crap, I think @eybarta is right. justify-content and align-items should be applied to the parent container instead of > *.

lost-align is broken when used with global flexbox or the flex param.

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Jan 13, 2016

Owner

Will look into this tonight.

Owner

peterramsing commented Jan 13, 2016

Will look into this tonight.

@adrian2x

This comment has been minimized.

Show comment
Hide comment
@adrian2x

adrian2x Jan 13, 2016

I just noticed this too. I thing @eybarta is right too. Unlike the position-based alignment, the flex alignment properties should be applied on the container element, because they specify how a flex element arranges its items. The way that it is implemented right now, it will apply these properties (justify-content and align-items) to the actual children, which will make them in turn flex containers, according to the spec.
PR -> #236

adrian2x commented Jan 13, 2016

I just noticed this too. I thing @eybarta is right too. Unlike the position-based alignment, the flex alignment properties should be applied on the container element, because they specify how a flex element arranges its items. The way that it is implemented right now, it will apply these properties (justify-content and align-items) to the actual children, which will make them in turn flex containers, according to the spec.
PR -> #236

@corysimmons

This comment has been minimized.

Show comment
Hide comment
@corysimmons

corysimmons Jan 14, 2016

Contributor

🎉

Contributor

corysimmons commented Jan 14, 2016

🎉

@eybarta

This comment has been minimized.

Show comment
Hide comment
@eybarta

eybarta Jan 14, 2016

glad this is being addressed
thanks guys 👍

eybarta commented Jan 14, 2016

glad this is being addressed
thanks guys 👍

@aguilera51284

This comment has been minimized.

Show comment
Hide comment
@aguilera51284

aguilera51284 Jun 28, 2016

i have the same problem here :)!

aguilera51284 commented Jun 28, 2016

i have the same problem here :)!

@peterramsing peterramsing modified the milestone: v8.0 Nov 6, 2016

@peterramsing peterramsing self-assigned this Nov 6, 2016

@peterramsing peterramsing referenced this issue Nov 12, 2016

Merged

Fixes Lost-align bugs #339

3 of 3 tasks complete

peterramsing added a commit that referenced this issue Nov 16, 2016

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Nov 16, 2016

Owner

Fixed here: #339

You'll see this in the beta version soon and in LostGrid v8. 👍

Owner

peterramsing commented Nov 16, 2016

Fixed here: #339

You'll see this in the beta version soon and in LostGrid v8. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment