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

The children items are not vertically 'stretched' properly when the height of the parent is defined. #78

Closed
prenaux opened this issue May 5, 2015 · 8 comments

Comments

@prenaux
Copy link
Contributor

prenaux commented May 5, 2015

The children items are not vertically 'stretched' properly when the height of the parent is defined.

in Chrome

in Chrome

in React Native

in React Native

Sample HTML used in Chrome

<style>
/*=== Normalize ========================================================*/
* { margin: 0; padding: 0; border: none; outline: none; }
* {
  -webkit-font-smoothing: subpixel-antialiased;
  -webkit-font-smoothing: antialiased;
}
html { font-size: 100%; overflow-y: scroll; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; }
body, button, input, select, textarea {
  font-family: "Helvetica Neue",Helvetica,Arial,sans-serif;
  color: #111;
}
html, body { height: 100%; width: 100%; margin: 0; }

/*=== Flex box everywhere ==============================================*/
div, span {
  background-color: #fff;

  box-sizing: border-box;
  position: relative;

  display: flex;
  flex-direction: column;
  align-items: stretch;
  flex-shrink: 0;

  border: 0 solid black;
  margin: 0;
  padding: 0;
}

/*=== Our Styles =======================================================*/
.container {
  background-color: #333333;
  width: 300px;
  height: 300px;
  flex-direction: row;
  flex-wrap: wrap;
}
.box {
  width: 50px;
  height: 50px;
  margin: 5px;
}
</style>

<div class="container">
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
  <div class="box"></div>
</div> 
@vjeux
Copy link
Contributor

vjeux commented May 5, 2015

Nice find, do you want to take a stab at fixing it?

@prenaux
Copy link
Contributor Author

prenaux commented May 5, 2015

Yup, I've got the basics working now. Hacking in the C code atm, I'll put it back to JS once I'm done.

I've added support for alignContent since that's the root of the problem. I'm referring to http://www.w3.org/TR/2012/CR-css3-flexbox-20120918/#align-content-property for the specs, its a bit light on math, but I believe I got the algo part sorted out now.

There's still an issue though, which is that I need an array - at least as far as I can tell now - to keep the size & nodes of the lines (on the crossAxis) so that I can re-distribute them afterward when you use alignContent to distribute them in stretch mode. I'll try to see if I can rework the algo without needing it, but per the spec - and as I've observed in Chrome/IE/FF, in the case of 'stretch' the result is computed 'after the fact', basically the new "row" height is a function of the computed size of the elements within the div (hopefully that makes sense ;)).

So, I probably need to add some "API" to allocate/free the temporary array needed, seems it shouldn't be too problematic. Either an external API or another "interface" function (like measure/get_dirty) in the css node...

If you have any suggestion in that regard let me know.

@vjeux
Copy link
Contributor

vjeux commented May 5, 2015

Instead of allocating an array, can you add another field on the node object that you are going to use while doing the update? This way you don't need to allocate arrays, it's already allocated?

@prenaux
Copy link
Contributor Author

prenaux commented May 5, 2015

Yes, it should be possible to do the same thing by just adding a line_index field to the css_node, and then recomputing what's necessary in the multi-line + alignContent with fixed size case. I suppose that's a better option since the code is basically allocation free atm.

@prenaux
Copy link
Contributor Author

prenaux commented May 6, 2015

Submitted a pull request with the implementation, going to integrate in react-native and add more test cases in Jasmine next.

@prenaux
Copy link
Contributor Author

prenaux commented May 6, 2015

alignContent is 'stretch' by default, same as alignItems and as Chrome/Specs. However the previous version of the code was behaving more like flex-start not sure if its better to be more like the previous behavior or to follow the specs...

@prenaux
Copy link
Contributor Author

prenaux commented May 6, 2015

A react component that 'tests' alignContent:

var React = require('react-native');
var {
  StyleSheet,
  Text,
  View,
  TouchableHighlight
} = React;

var styles = StyleSheet.create({
  container: {
    backgroundColor: '#333333',
    width: 300,
    height: 380,
    flexDirection: 'row',
    flexWrap: 'wrap',
    padding: 3,
  },
  box: {
    backgroundColor: '#ffffff',
    width: 50,
    height: 50,
    margin: 10,
  },
  box2: {
    backgroundColor: '#ffffff',
    width: 50,
    height: 100,
    margin: 10,
  },
  lighter_grey: {
    backgroundColor: '#606060',
  },
  red: {
    backgroundColor: '#ff0000',
  },
  green: {
    backgroundColor: '#00ff00',
  },
  cyan: {
    backgroundColor: '#00ffff',
  },
  button: {
    margin: 5,
    fontWeight: 'normal',
  },
  selected: {
    fontWeight: '800',
  },
});

module.exports = React.createClass({
  getInitialState: function() {
    return {
      alignItems: 'flex-start',
      alignContent: 'stretch'
    };
  },

  alignItems: function(aNewAlignItems) {
    this.setState({
      alignItems: aNewAlignItems
    });
  },

  alignContent: function(aNewAlignContent) {
    this.setState({
      alignContent: aNewAlignContent
    });
  },

  render: function() {
    var button = (aName,aKind) => {
      return (
        <TouchableHighlight onPress={() => { this[aKind](aName) } }>
          <Text style={[
                styles.button,
                this.state[aKind] == aName ? styles.selected : undefined
                ]}>
            {aName}
          </Text>
        </TouchableHighlight>
      );
    }

    return (
      <View>
        <View style={
              [styles.container,{
               alignItems: this.state.alignItems,
               alignContent: this.state.alignContent
              }]}>
          <View style={[styles.box,styles.red]}><Text>0</Text></View>
          <View style={[styles.box,styles.red]}><Text>1</Text></View>
          <View style={[styles.box,styles.red]}><Text>2</Text></View>
          <View style={[styles.box,styles.red]}><Text>3</Text></View>
          <View style={[styles.box2,styles.red]}><Text>4</Text></View>
          <View style={[styles.box,styles.green,{alignSelf:'flex-start'}]}><Text>5</Text></View>
          <View style={[styles.box,styles.green]}><Text>6</Text></View>
          <View style={[styles.box2,styles.green]}><Text>7</Text></View>
          <View style={[styles.box,styles.green]}><Text>8</Text></View>
          <View style={[styles.box,styles.green]}><Text>9</Text></View>
          <View style={[styles.box,styles.cyan,{alignSelf:'flex-start'}]}><Text>10</Text></View>
          <View style={[styles.box,styles.cyan]}><Text>11</Text></View>
          <View style={[styles.box,styles.cyan]}><Text>12</Text></View>
          <View style={[styles.box,styles.cyan,{alignSelf:'flex-start'}]}><Text>13</Text></View>
          <View style={[styles.box,styles.cyan,{alignSelf:'stretch'}]}><Text>14</Text></View>
        </View>
        <View style={{flexDirection:'row'}}>
          <View style={{flex:0.5}}>
            <Text>AlignItems:</Text>
            {button('flex-start','alignItems')}
            {button('center','alignItems')}
            {button('flex-end','alignItems')}
            {button('stretch','alignItems')}
          </View>
          <View style={{flex:0.5}}>
            <Text>AlignContent:</Text>
            {button('flex-start','alignContent')}
            {button('center','alignContent')}
            {button('flex-end','alignContent')}
            {button('stretch','alignContent')}
          </View>
        </View>
      </View>
    );
  }
});

vjeux added a commit that referenced this issue May 17, 2015
[Issue #78]: Implemented alignContent ;
@prenaux
Copy link
Contributor Author

prenaux commented May 17, 2015

#79 has been merge, so closing this.

@prenaux prenaux closed this as completed May 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants