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 layout elements for the new XY-Grid component #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add layout elements for the new XY-Grid component #35

wants to merge 1 commit into from

Conversation

jeromelebleu
Copy link

Foundation v6.4 introduces a new XY-Grid component, see here.

This adds the GridX, GridY and Cell elements with only basics classes. Should I also add a GridContainer class to handle the Grid Container?

Thanks in advance - and for your work by the way!

@sveetch
Copy link
Owner

sveetch commented Aug 2, 2017

Ah, i was afraid this moment someone will ask about GridXY because i'm not a huge fan about this component so i totally dropped out my mind :)

But i can't refuse it, the only thing i need it's a test to validate it so i can reasonably keep it without to be broken on future changes. I recommend you to follow the development doc then add your own as something like "006_grid.py", copying and adapting this test to your layout object would be enough, then i will review it before merging it and document it.

For the grid container, i can't really say since i don't use gridXY, but it seems useful no? I may say first finish the test for what you already did, think about it and open up a new dedicated PR.

@jeromelebleu
Copy link
Author

Ah, i was afraid this moment someone will ask about GridXY because i'm not a huge fan about this component so i totally dropped out my mind :)

Oops, sorry to be that guy... :) Out of curiosity, what do you not like in this component?

But i can't refuse it, the only thing i need it's a test to validate it so i can reasonably keep it without to be broken on future changes. [...]

No problem, I will work on that. This PR was more a starting point to discuss it, get your feedbacks and to know how to code that. Thanks for your recommendations, I will try to add some tests first so!

@ghost
Copy link

ghost commented Oct 22, 2017

I was pretty certain I would not like the GridXY - but I now love it. Simplicity.

I would love to see it added. The best I can do now is something crazy like:

Div ( 
    Div ( 'email', css_class = 'large-8 large-offset-2 cell' ),
    css_class = 'grid-x'
    ),

I think XY is here to stay and am now a fan. So this would be a very welcome addition for me.

thanks for the great work.

@sveetch
Copy link
Owner

sveetch commented Nov 8, 2017

I would need some help on this, since i need some tests to validate GridXY support and for now i don't have time to study gridxy references.

@jeromelebleu
Copy link
Author

I have finally not used crispy forms in my application... nor crispy-forms-foundation. As I don't plan to use it soon, I will not be able to help you in adding tests - and ensure that it works! - sorry about that.

@ghost
Copy link

ghost commented Nov 13, 2017 via email

@jeromelebleu
Copy link
Author

I would be happy to help, but I am not a professional coder.

Feel free to ask, if I can help you! And don't worry, you don't have to be a professional coder... My only issue is that I will not use it...

@ghost
Copy link

ghost commented Dec 7, 2017 via email

@mpasternak
Copy link
Contributor

@sveetch I'm pretty new to Foundation 6, but basically "row" becomes "grid-x" or "grid-x grid-margin-x" if you want spaces and "columns" becomes "cell". "large-N", "small-N", "medium-N" classes stay the same.

This upgrade (to GridXY) is required, because everything else becomes obsolete in the grid dept with the next foundation release. But, I assure you, there's nothing to be afraid of. At least it looks like it.

@sveetch
Copy link
Owner

sveetch commented Feb 26, 2018

No problemo, the only thing required is to add gridXY support as an alternative, float grid still be basic way from Foundation<7, and that according unittest is added and run nicely.

@ryancausey
Copy link

What needs to be done to this PR, besides fixing merge conflicts, in order for it to be merged?

Or, is there another plan in the works to add XY Grid support?

@sveetch
Copy link
Owner

sveetch commented Dec 3, 2018

@ryancausey it lacks of unit tests on added layout elements.

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

Successfully merging this pull request may close these issues.

4 participants