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

Lost-column/waffle float:right to last cycle unexpected outcome #328

Closed
akoury opened this issue Sep 28, 2016 · 9 comments
Closed

Lost-column/waffle float:right to last cycle unexpected outcome #328

akoury opened this issue Sep 28, 2016 · 9 comments

Comments

@akoury
Copy link

@akoury akoury commented Sep 28, 2016

Is this a feature request or a bug report?

Bug Report

What is the current behavior?

Regarding issue #200 which was solved in pull request #218 , a float:right was added to the last cycle of a column/waffle, which leads to a wrong display of elements

If it's a bug please provide the steps to reproduce it and maybe some code samples.

Just add any number of elements in a view, place a "lost-column: 1/4 2" (or a lost-waffle, with a cycle smaller than the denominator) and look at the outcome, the second "column" of elements gets pushed to the right side of the screen due to the float, instead of leaving the space that was specified in the 2nd cycle. This worked perfectly in versions prior to 6.7.0.

What is the behavior that you expect?

The behaviour present before 6.7.0, in which cycles dont get pushed to the right

What's the motivation or use-case behind changing the behavior?

Restore this great grid system's behaviour

What version of LostGrid, browser and browser version are affected by this issue? Did this happen in previous versions?

I checked it in Safari and Chrome on Mac, It did not happen before 6.7.0.

Anything else?
If this was done on purpose and you just wanted to change the way the grid works, then dont mind me (I just think that it was better before).

This is my html/scss

screen shot 2016-09-27 at 8 44 21 pm

This is the outcome

screen shot 2016-09-27 at 8 38 19 pm

If i remove the float:right to the last cycle, this happens (what i believe should happen)

screen shot 2016-09-27 at 8 38 48 pm

I am not doing a pull request myself removing the float:right from the source because that would simply re-open issue #200

@peterramsing
Copy link
Owner

@peterramsing peterramsing commented Sep 28, 2016

@akoury I see where you're going with this. As a people pleaser, doing Open Source work on large projects is really tough. I want everyone happy.

That said, thank you so much for this issue! You're the type of person that makes Open Source really fun! This is a really well thought out issue.

If I knew this would cause any breaking changes I would have certainly done #218 as a major release. I'm sorry.

My first thought is to make this a option for lost-waffle so that you could specify whether or not you want the float in place.

@akoury
Copy link
Author

@akoury akoury commented Sep 29, 2016

@peterramsing please dont apologize! your work here has been great and you are not even getting compensated for this, so know that everything you do is greatly appreciated.

Regarding the issue: like i said, i could've made a pull request just removing the float right from both lost-waffle and lost-column files, however i think that would just make the previous issue #200 reappear, so i dont know if a toggeable option would be a perfect fit.

Sorry that i don't have much to contribute regarding ideas on how to solve it, I am very new to this field, maybe @corysimmons has some ideas on what could be done hehe

(I tried removing the float:right for testing and i see that issue #200 reappears, even in Chrome, not just Safari :( )

@peterramsing
Copy link
Owner

@peterramsing peterramsing commented Sep 29, 2016

I'll look into a param and some more testing in the waffle's case.

@corysimmons
Copy link
Contributor

@corysimmons corysimmons commented Sep 29, 2016

@peterramsing Why don't we break out all the params for these various things into their own properties (as well as keeping shorthand)?

Lost keeps needing more params to help with various use cases, but lost-column: a b c d e f g is pretty gnarly whereas:

lost-column-fraction: 1/4;
lost-column-gutter: 45px;
lost-column-cycle: 3;

etc... is a lot more readable/composable.

It'd let us tack these features on left and right (after discussing their value as a param of course).

Really, the only way I see to address the plethora of use-cases is to add more params/settings/etc. /shrug

@peterramsing peterramsing modified the milestone: v8.0 Nov 6, 2016
@peterramsing peterramsing self-assigned this Nov 6, 2016
peterramsing added a commit that referenced this issue Nov 13, 2016
@peterramsing
Copy link
Owner

@peterramsing peterramsing commented Nov 16, 2016

I think that I'm going to remove the float as standard for lost-waffle with a param for if the float should be used. @akoury, I think you're right when you said that the old way was better and I'm sorry that it changed and I didn't realize it would cause that much change.

You'll see this change in LostGrid v8.

@peterramsing
Copy link
Owner

@peterramsing peterramsing commented Nov 16, 2016

@akoury Here's the fix: #343

I'm going to add in some more properties for the shorthand properties. Probably should make all shorthands have a corresponding longhand.

@akoury
Copy link
Author

@akoury akoury commented Nov 16, 2016

@peterramsing two questions:

  1. Basically the default will return to normal and if issue #200 bothers us we can use the float right param for that specific waffle?
  2. Does this also apply to lost-column?

Seems great so far! thanks for the awesome help

@peterramsing
Copy link
Owner

@peterramsing peterramsing commented Nov 16, 2016

That's correct.

As for lost-column, I was thinking that it would retain the float as that's a more typical use case for the column.

@akoury
Copy link
Author

@akoury akoury commented Nov 16, 2016

@peterramsing perfect! thanks again for doing such an awesome work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.