idibidiart / react-native-responsive-grid

Bringing the Web's Responsive Design to React Native
Other
379 stars 42 forks source link

Col doesn't work as expected with breakpoints #6

Closed millersdev closed 7 years ago

millersdev commented 7 years ago

When using the following code;

<Row>
    <Col style={{ backgroundColor: 'green' }} smSize={50} mdSize={33.333} lgSize={25}>
        <Text>First Column</Text>
    </Col>
    <Col style={{ backgroundColor: 'green' }} smSize={50} mdSize={33.333} lgSize={25}>
        <Text>Second Column</Text>
    </Col>
    <Col style={{ backgroundColor: 'green' }} smSize={50} mdSize={33.333} lgSize={25}>
        <Text>Third Column</Text>
    </Col>
    <Col style={{ backgroundColor: 'green' }} smSize={50} mdSize={33.333} lgSize={25}>
        <Text>Fourth Column</Text>
    </Col>
</Row>

The third and fourth column are not visible on iphone portrait, and the fourth column is not visible on iphone landscape.

I expect that this behaviour needs to be the same as the bootstrap way..?

idibidiart commented 7 years ago

@millersdev

iPhone size falls under 'sm' and 50% plus 50% is 100% so that's the entire width, and since Row has no 'wrap' the third and fourth columns will be off screen. On landscape, the screen size is no longer 'sm' but 'md' so you'll see three columns since each are 1/3rd or 33.333%

What is the Bootstrap behavior you're trying to mimic? If you wish to break into two lines when in portrait mode (two columns in each line) then you must supply the 'wrap' prop on the row (does not have to have any value, it's just a Boolean)

image0 image1

So 'wrap' is required and unlike Bootstrap is not set by default.

This grid for better or worse is not trying to emulate any other grid or standard, but it attempts to be as close as possible to the Flexbox standard, except where the latter introduces unnecessary confusion and inconsistency, which this grid attempts to pave over. This grid tries to be intuitive from an axiomatic perspective, for users who know nothing about grids. That is to say that a Row has no unexpected behavior such as wrapping unless that behavior is specified by the user. Else, it's just a Flexbox with flexDirection set to 'row.'

I'll close this, but if you feel strongly about 'wrap' being default, feel free to give the rationale for it.

Thanks

idibidiart commented 7 years ago

Made a note under 'wrap' prop in README.md to alert Bootstrap users to this fact.

millersdev commented 7 years ago

Thanks for your clear explanation.

I do already understand the sizes and breakpoints and why they fall out of the screen and when sm, md, lg etc will be active but still okay for future purpose to explain it.

I overlooked this prop, i thought it was a bug because i would expect this as default behaviour, i cant imagine a situation where you want defined columns to go out of the screen, especially when you talk about a "grid".

This is not only bootstrap behaviour so i think more people will struggle with this prop.

Because RN is attractive for web developers i would say that wrap is default behaviour and "unwrap" if necessary is optional.

idibidiart commented 7 years ago

You might be right. Let me think about it. Thank u for bringing this up...

idibidiart commented 7 years ago

@millersdev

I thought about it and I found only one use case where 'wrap' be default would be confusing, which is where you're building something like a carousel and you have elements off screen that you transition sideway intentionally to create the carousel effect. This is generally the case for horizontal scrolling.

Given that's just a very small use case and I could not come up with any other reason I've decided to make 'wrap' the default behavior as you proposed.

Next release is coming soon, with some cleanup and a new demo :)