play-co / timestep

GNU General Public License v3.0
16 stars 27 forks source link

Fixed `color` property inheritance issue. #75

Closed jsdevlover closed 8 years ago

jsdevlover commented 8 years ago

Base color property was not inherited correctly because of a missing entry in BASE_STYLE_PROPS.

rogueSkib commented 8 years ago

@jsdevlover thanks for the PR, can you explain the issue you were running into? What class are you using that needs style.color? We use style.backgroundColor, which is already supported across all platforms. How is this property different? Cheers!

jsdevlover commented 8 years ago

Hey @rogueSkib , Apologies for replying so late.

Here is sample Application.js which would explain what issue I'm facing.. Please have a look at the comments in the code. In short color property is not inherited the way backgroundColor is. Hope this makes it clear now.

import ui.TextView as TextView;
import ui.View as View;

exports = Class(GC.Application, function() {

  this.initUI = function() {

    this.outerView = new View({
      superview: this.view,
      backgroundColor: 'orange',
      color: 'white',
      x: 100,
      y: 100,
      width: this.view.style.width,
      height: 500
    });

    new TextView({
      superview: this.outerView,
      text: 'Hello, world!',
      /*
        After commenting out below line, TextView inherits backgroundColor from 
        outerView which is orange.
      */
      //backgroundColor: 'green',
      /*
        But here the text is displayed in black color and not white which 
        it should inherit from outerView.
      */
      //color: 'blue',
      x: 100,
      y: 100,
      width: this.view.style.width,
      height: 100
    });

  };

  this.launchUI = function() {

  };

});
rogueSkib commented 8 years ago

@jsdevlover thanks for the sample code! The TextView in your example has a transparent backgroundColor by default, so the orange from the parent view is showing through; the backgroundColor property is not shared with child views. If you uncomment the backgroundColor: 'green', line and change the x value of the TextView to -50, you will see its green rectangle breaking out of the left side of its parent's orange rectangle. If you then remove the green, you will see that the orange is not inherited by the child view and it defaults back to transparent on top of the orange.

I also tried using the change in this PR, but it didn't have any affect on the sample code provided. I hope this helps clear up any confusion you might have had; I'll close this issue for now because it looks like everything is working as intended.

jsdevlover commented 8 years ago

@rogueSkib , Thanks for the detailed explanation. I'll have a look at my code again to check if anything else is wrong.