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

Unexpected behavior when modifying the config and props of children. #149

Closed
ethib137 opened this issue Aug 8, 2016 · 1 comment
Closed
Assignees
Labels

Comments

@ethib137
Copy link

ethib137 commented Aug 8, 2016

We have a number of instances where we modify the children of a component before we render the children. This allows us to add props to those children or at times conditionally render children.

This issue I'm noticing is that modifying the props of children is handled differently if the child is a metal component or an html element.

When I modify the props of a child that is an html element the props are appropriately applied to the dom. When I modify the props of a child that is a metal component, that component instance does not show those props existing.

On the other hand if I modify the config of a child that is an html element, the attributes passed as config are not rendered in the dom. And when I modify the config of a child that is a metal component, that component instance does the attributes passed through config on this.props.

The example below displays this issue:

The click event can only be passed to a child if it is passed as the props of an html element, or as the config of a metal component.

import Component, {Config} from 'metal-jsx';

const styles = {
    cursor: 'pointer',
    padding: '20px'
};

class MetalComponent extends Component {
    render() {
        const {children, ...other} = this.props;

        return <div {...other}>{this.props.children}</div>;
    }
}

class ClickComponent extends Component {
    created() {
        this.handleClick_ = this.handleClick_.bind(this);
    }

    handleClick_() {
        console.log('handleClick_', this.props.objectKey);
    }

    render() {
        const {children, objectKey} = this.props;

        const trigger = children[0];

        trigger[objectKey] = {
            ...trigger[objectKey],
            onClick: this.handleClick_
        };

        return <div style={styles}>{trigger}</div>;
    }
}

ClickComponent.PROPS = {
    objectKey: Config.string()
};

class Test extends Component {
    render() {
        return (
            <div>
                <ClickComponent objectKey="props">
                    <div>{'onClick div, pass through props'}</div>
                </ClickComponent>

                <ClickComponent objectKey="config">
                    <div>{'onClick div, pass through config'}</div>
                </ClickComponent>

                <ClickComponent objectKey="props">
                    <MetalComponent>{'onClick metal component, pass through props'}</MetalComponent>
                </ClickComponent>

                <ClickComponent objectKey="config">
                    <MetalComponent>{'onClick metal component, pass through config'}</MetalComponent>
                </ClickComponent>
            </div>
        );
    }
}

Test.STATE = {};

export default Test;

Thanks.

@mairatma
Copy link
Contributor

mairatma commented Aug 8, 2016

You're right, there were a few places that were still using the config object instead of the new props one, so if you manually reset props this wouldn't work. The main object should be props.

We'll later add a utility function for changing a child's props, so that you don't need to manually change them like this. This function will also help make sure that both config and props are changed every time, so that they can both be used with no data consistency problem.

@mairatma mairatma self-assigned this Aug 8, 2016
@mairatma mairatma added the bug label Aug 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants