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

clear: left on element after cycle #276

Closed
schelmo opened this Issue Apr 14, 2016 · 17 comments

Comments

Projects
None yet
3 participants
@schelmo

schelmo commented Apr 14, 2016

Example:
http://codepen.io/schlmm/pen/PNRBZw?editors=1111
shouldnt the third element define clear: right (or even both) instead of clear: left, since the second element hast float: right ?
as you can see the third element doesnt start in a new "row".
if you change clear: left, to clear: right, the behaviour would be the same as if you use lost with flexbox.
(with flex: https://codepen.io/schlmm/pen/KzoBWq )

@pl-mnm

This comment has been minimized.

Show comment
Hide comment
@pl-mnm

pl-mnm Apr 18, 2016

I'm having the same issue, clear: right does it for me however I can't figure out why it's working in the demos.

pl-mnm commented Apr 18, 2016

I'm having the same issue, clear: right does it for me however I can't figure out why it's working in the demos.

@schelmo

This comment has been minimized.

Show comment
Hide comment
@schelmo

schelmo Apr 18, 2016

looks like it only happen if you have 2 columns in a row.
lost-column: 1/3 works: http://codepen.io/schlmm/pen/MyGxMV
why the last element must be "float: right" ?

schelmo commented Apr 18, 2016

looks like it only happen if you have 2 columns in a row.
lost-column: 1/3 works: http://codepen.io/schlmm/pen/MyGxMV
why the last element must be "float: right" ?

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Apr 19, 2016

Owner

@schelmo Thanks submitting this. I think you're onto something here regarding how there are differences in using flexbox and not within Lost but could you be a bit more specific as to what you would expect it to do?

Regarding the reason for adding float: right. See: #218

@pl-mnm Could you elaborate on how clear: right does it for you? What does it do?

@schelmo Could you be a bit more specific on what you would expect this to do?

Owner

peterramsing commented Apr 19, 2016

@schelmo Thanks submitting this. I think you're onto something here regarding how there are differences in using flexbox and not within Lost but could you be a bit more specific as to what you would expect it to do?

Regarding the reason for adding float: right. See: #218

@pl-mnm Could you elaborate on how clear: right does it for you? What does it do?

@schelmo Could you be a bit more specific on what you would expect this to do?

@pl-mnm

This comment has been minimized.

Show comment
Hide comment
@pl-mnm

pl-mnm Apr 19, 2016

Apologies for not being clear the first time around. I'm having the same issue as described above (also in the codepen): in a two column grid the first cell in the second row doesn't clear anything so the top of the cells don't align. When I open up firebug, editing the clear: left to right manually works and the cells align nicely. Am I missing something?

pl-mnm commented Apr 19, 2016

Apologies for not being clear the first time around. I'm having the same issue as described above (also in the codepen): in a two column grid the first cell in the second row doesn't clear anything so the top of the cells don't align. When I open up firebug, editing the clear: left to right manually works and the cells align nicely. Am I missing something?

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Apr 19, 2016

Owner

@pl-mnm By "aligning" do you mean being flush with the bottom of the first "row"?

Owner

peterramsing commented Apr 19, 2016

@pl-mnm By "aligning" do you mean being flush with the bottom of the first "row"?

@pl-mnm

This comment has been minimized.

Show comment
Hide comment
@pl-mnm

pl-mnm Apr 19, 2016

Yes that's correct.

pl-mnm commented Apr 19, 2016

Yes that's correct.

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Apr 19, 2016

Owner

@pl-mnm I attached a screenshot of what I'm understanding from you.

screen shot 2016-04-18 at 8 11 21 pm

Owner

peterramsing commented Apr 19, 2016

@pl-mnm I attached a screenshot of what I'm understanding from you.

screen shot 2016-04-18 at 8 11 21 pm

@pl-mnm

This comment has been minimized.

Show comment
Hide comment
@pl-mnm

pl-mnm Apr 19, 2016

Yes that's it. Sorry I didn't do it myself, I thought the codepen would be enough!

pl-mnm commented Apr 19, 2016

Yes that's it. Sorry I didn't do it myself, I thought the codepen would be enough!

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Apr 19, 2016

Owner

I'm also seeing what you mean that the "desired" behaviour happens when lost-column: 1/3 is used.

I think I'm understanding a bit better now. I'll investigate.

Owner

peterramsing commented Apr 19, 2016

I'm also seeing what you mean that the "desired" behaviour happens when lost-column: 1/3 is used.

I think I'm understanding a bit better now. I'll investigate.

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Apr 19, 2016

Owner

@pl-mnm The codepen was really helpful but one used Flexbox and the other didn't so I just wanted to be sure we were on the same page. 😄

Owner

peterramsing commented Apr 19, 2016

@pl-mnm The codepen was really helpful but one used Flexbox and the other didn't so I just wanted to be sure we were on the same page. 😄

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Apr 19, 2016

Owner

I think the issue is confined to having having the cycle of 2. That is for 1/2 or anything with the cycle specified as 2.

Owner

peterramsing commented Apr 19, 2016

I think the issue is confined to having having the cycle of 2. That is for 1/2 or anything with the cycle specified as 2.

@schelmo

This comment has been minimized.

Show comment
Hide comment
@schelmo

schelmo Apr 20, 2016

if we change it to "clear:right", we get a similar floating issue with a cycle greater than 2
http://codepen.io/schlmm/pen/grKoNx
so "clear:both" is the way to go ?

schelmo commented Apr 20, 2016

if we change it to "clear:right", we get a similar floating issue with a cycle greater than 2
http://codepen.io/schlmm/pen/grKoNx
so "clear:both" is the way to go ?

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing Apr 20, 2016

Owner

@schelmo Worth a try. I need to address #262 first and then can tackle this.

Owner

peterramsing commented Apr 20, 2016

@schelmo Worth a try. I need to address #262 first and then can tackle this.

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing May 23, 2016

Owner

@schelmo My gut is saying this is a breaking change. While it is fixing a "bug", I worry that there are people that are relying on this by accident.

A thought might be that this could be a feature flag that could be enabled.

Owner

peterramsing commented May 23, 2016

@schelmo My gut is saying this is a breaking change. While it is fixing a "bug", I worry that there are people that are relying on this by accident.

A thought might be that this could be a feature flag that could be enabled.

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing May 23, 2016

Owner

@schelmo I'm working on adding a rule that would enable reverting to the old way with a global setting so that a refactor of code isn't needed if this was relied on. Should have something to glance at tonight.

Owner

peterramsing commented May 23, 2016

@schelmo I'm working on adding a rule that would enable reverting to the old way with a global setting so that a refactor of code isn't needed if this was relied on. Should have something to glance at tonight.

peterramsing added a commit that referenced this issue May 23, 2016

Adds global variable for clearing for fallback. | #276
If a user would want to use the previous clearing method, this allows
this.

peterramsing added a commit that referenced this issue May 23, 2016

@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing May 23, 2016

Owner

Closing for PR:
cc: @schelmo @pl-mnm
#293

Owner

peterramsing commented May 23, 2016

Closing for PR:
cc: @schelmo @pl-mnm
#293

peterramsing added a commit that referenced this issue May 31, 2016

Updates the "clear" rule from "left" to "both" (#297)
* Adds global variable for clearing for fallback. | #276
  * If a user would want to use the previous clearing method, this allows
this.
* Adds support for the new clearing method of "both" to lost-column. | #276
* Resets the globals in tests after lost-column test changes it.
* Adds support for the new clearing method of "both" to lost-waffle. | #276
@peterramsing

This comment has been minimized.

Show comment
Hide comment
@peterramsing

peterramsing May 31, 2016

Owner

@schelmo @pl-mnm This is now merged into the beta branch. https://github.com/peterramsing/lost/releases/tag/v7.0.0-beta.1

Also on npm. npm install --save lost@beta

Owner

peterramsing commented May 31, 2016

@schelmo @pl-mnm This is now merged into the beta branch. https://github.com/peterramsing/lost/releases/tag/v7.0.0-beta.1

Also on npm. npm install --save lost@beta

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