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

Flexbox grid support #169

Closed
madisvain opened this issue Oct 5, 2016 · 7 comments
Closed

Flexbox grid support #169

madisvain opened this issue Oct 5, 2016 · 7 comments

Comments

@madisvain
Copy link

Currently Col component does not support flexbox as there is no way to define a without it having the column number.

http://v4-alpha.getbootstrap.com/layout/flexbox-grid/
Unlike the default grid system where a grid column starts as full-width in the xs tier, flexbox requires a .col-{breakpoint} class for each tier.

Flexbox would require to have the possibility of defining <div class="col-xs"> but currently the element will add col-xs-12 by default.

@eddywashere
Copy link
Member

Thanks for filing the issue! I could imagine leveraging the size props, and adding a boolean option that would only output the col-${size} class. It would look like this:

<Row>
  <Col xs>col-xs</Col>
  <Col xs="6">col-xs-6</Col>
  <Col xs>col-xs</Col>
</Row>

Are the push/pull/offset classes supported in flexbox grid? If so, maybe flex is a prop or the size props accept flex or auto as a way to output the auto layout class.

<Row>
  <Col xs="flex">col-xs</Col>
  <Col xs="6">col-xs-6</Col>
  <Col xs={{offset: 1, size: 'flex'}}>col-xs</Col>
</Row>
<Row>
  <Col xs="auto">col-xs</Col>
  <Col xs="6">col-xs-6</Col>
  <Col xs={{offset: 1, size: 'auto'}}>col-xs</Col>
</Row>

eddywashere pushed a commit that referenced this issue Oct 18, 2016
* feat(Col): add flex-support for Col component (#169)

* feat(Col): Removed "flex" as valid option for col size, and bool for object (#169)

* feat(Col): add documentation for flexbox support (#169)
@ronnyhaase
Copy link
Contributor

ronnyhaase commented Oct 18, 2016

@ronnyhaase
Copy link
Contributor

Hey guys!

I'm almost done with the last task for full flexbox support: Support flex alignment classes of Bootstrap.

I'd like to ask you for feedback regarding naming and types for the necessary properties:

Context: http://v4-alpha.getbootstrap.com/layout/flexbox-grid/

My current implementation looks like:

<Col first="xs" first={['xs', 'sm']} first={{ xs: true, sm: true, /* ... */ }} first />

Where a boolean first could may resolve to ['xs', 'sm', 'md', 'lg', 'xl'] and/or accept "all"?

What do you think about it? And also, do you think the props should be prefixed, like order-first. Or is it better to just have one prop order which expects first, last and unordered as attributes?

<Col order={ first: ['xs'], last: ['xs', 'sm'], unordered: ['lg', 'xl'] } />

Or maybe both???

For <Row /> the same question arrises for horizontal & vertical alignment.

<Row top middle bottom left center right around between />

and/ or (?)

<Row alignVertical={{...}} alignHorizontal={{...}} />

or even (?)

<Row alignment={ vertical: { middle: ['xs', 'sm'] }, horizontal: ... } />

Looking forward to your feedback!

@ronnyhaase
Copy link
Contributor

My personal preference right now would be single props accepting string or array of string, and bool / 'all' for all sizes.

@matias-casal
Copy link

@ronnyhaase first, thanks for all, i really appreciate it.
You finally decided how to do the vertical alignment?

@eddywashere
Copy link
Member

@ronnyhaase thanks again for your patience and hard work on these components. Following up on our chat, here's the suggestion I mentioned for Col & Row.

<Col xs xsFirst xsLast xsUnordered></Col>

and

<Row xs xsLeft xsTop></Row>

It adds more props but I think it simplifies the usage.

@eddywashere
Copy link
Member

bootstrap 4 has switched to flexbox. #267 updates Col component to support new classes. Also, as a result of the flexbox switch, there were new flexbox utility classes added to bootstrap. Instead of making these props available on the components that could potentially use it, I think the simple solution is to let users add those classes manually, ex: className="justify-content-between".

I'm going to close this out as I think the v4 alpha branch of reactstrap should satisfy the flexbox needs.

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

No branches or pull requests

4 participants