three-mesh-ui icon indicating copy to clipboard operation
three-mesh-ui copied to clipboard

Properties are inconsistencely propagated from block to children

Open swingingtom opened this issue 3 years ago • 3 comments

I made a codesandbox that shows a gui will all block properties available. https://codesandbox.io/s/tmu-properties-overview-zeggq

I choose a codesandbox in order to provide this "non-example" / "non-sample" piece of code. As @hybridherbst said in #129 , we may lack a space to produce, edit, run tests or development parts without polluting the "examples" folder which is published.

Block correctly propagates its properties to children upon construction. But through update, properties propagation behaves inconsistencely according to property type.

fontSize

Updating fontSize on parent DONT update children. ( changing the fontFamily afterward produce an update that also update that took the correct fontSize value)

Once child fontSize is set, setting block fontSize has no more impact on child.

letterSpacing

Setting letterSpacing on parent DO update children.

Once child fontSize is set, setting block letterSpacing has no more impact on child.

*Colors

Setting *Color on parent DO update children.

EVEN when child *Color is set, setting block *color IMPACTS children

backgroundOpacity

backgroundOpacity has a default value of undefined, even when

new ThreeMeshUI.Block({
    backgroundOpacity: 0
});

To conclude

Im not for or against any behaviour, but it is currently difficult to know how it will behave.

swingingtom avatar Jan 20 '22 20:01 swingingtom

we may lack a space to produce, edit, run tests or development parts without polluting the "examples" folder which is published.

I'm mostly worried that we will duplicate most of what is in examples/assets and examples/utils... If we create a space for work-in-progress experiments I think it should still be in the example folder. Maybe an example/WIP folder ?

About the propagation of attributes yes it's inconsistent, or at least not documented enough.

Here is the thing: for some attributes like fontSize, it's common that you set it on the root and want all children to inherit. For some others like width it's the reverse, it's obvious it shouldn't propagate. It's also for convenience, requiring users to set all attributes on every component is quite verbose.

An alternative to the current behavior would be to propagate nothing, but create inherit values for attributes like fontSize.

felixmariotto avatar Jan 20 '22 23:01 felixmariotto

An alternative to the current behavior would be to propagate nothing, but create inherit values for attributes like fontSize.

Is this something you already thought before? I think it could be a good move, it mimicks css, and if we split (I don't know how) Defaults.js to have separated values for Block and Text.

export default  {
        block: {
	    fontSize: 0.05,
	    interLine: 0.01,
	    letterSpacing: 0
        },
        text: {
           fontSize: 'inherit',
           letterSpacing: 'inherit',
        }
};

We may be able to provide a minor release.

swingingtom avatar Jan 21 '22 07:01 swingingtom

I'm mostly worried that we will duplicate most of what is in examples/assets and examples/utils... If we create a space for work-in-progress experiments I think it should still be in the example folder. Maybe an example/WIP folder ?

Moved to discussion #116

swingingtom avatar Jan 21 '22 08:01 swingingtom