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

Reparenting of Space element results into remounting of children elements #31

Closed
thekapadia opened this issue Oct 16, 2019 · 3 comments
Closed

Comments

@thekapadia
Copy link

@aeagle This issue is related to the way Layer is currently implemented and facebook/react#3965.

Let me try and explain below.

Say, I have the following structure.

<Space.Fixed>
    <Space.Top size={300} />
    <Space.Fill />
    <Space.Bottom size={300} />

    <Space.Layer zIndex={1}>
        <Space.Top size={100}>
            <MyComponent />
        </Space.Top>
    </Space.Layer>
</Space.Fixed>

Now for some reason, I have to reparent the <Space.Top size={100}> component as shown below.

<Space.Fixed>
    <Space.Top size="300" order={1} />
    <Space.Top size={100} order={2}>
        <MyComponent />
    </Space.Top>
    <Space.Fill />
    <Space.Bottom size="300" />
</Space.Fixed>

So I removed <Space.Top size={100}> from the Layer and added it to the base structure.

When I do this, <MyComponent /> is destroyed and recreated entirely because of facebook/react#3965. This hits the performance of <MyComponent /> and also results in loss of its state.

I was thinking if React Spaces could additionally support layering by avoiding nesting, maybe by passing in a prop, something like below.

<Space.Fixed>
    <Space.Top size={100} layered={true} zIndex={1}>
        <MyComponent />
    </Space.Top>
    <Space.Top size="300" />
    <Space.Fill />
    <Space.Bottom size="300" />
    <Space.Bottom size={100} layered={true} zIndex={2}>
        <MyComponent />
    </Space.Bottom>
</Space.Fixed>

Thoughts?

@aeagle
Copy link
Owner

aeagle commented Oct 16, 2019

@thekapadia There is actually a zIndex property on the spaces already which you can use with having to use the Layer wrapper. However, I'm pretty sure that the behaviour when changing that property (i.e. moving a space between different layers) will not work as expected - I will take a look at that. Give it a go anyway.

@thekapadia
Copy link
Author

@aeagle Right, the zIndex prop helped. Without wrapping into the Layer component.

                <Fixed height={400}>
                    <Left size={200} zIndex={1} style={{ background: 'yellow' }}>
                        <p> Some component here </p>
                    </Left>
                    <Left size={300} style={{ background: 'pink' }}>
                        <p> Some component here </p>
                    </Left>
                    <Fill style={{ background: 'cyan' }}>
                        <Top size={100} zIndex={1} style={{ background: 'purple' }}>
                            <p> Some component here </p>
                        </Top>
                        <Bottom size={100} zIndex={2} style={{ background: 'purple' }}>
                            <p> Some component here </p>
                        </Bottom>
                    </Fill>
                    <Right size={300} style={{ background: 'pink' }}>
                        <p> Some component here </p>
                    </Right>
                    <Right size={200} zIndex={2} style={{ background: 'yellow' }}>
                        <p> Some component here </p>
                    </Right>
                    <Right size={100} zIndex={3} style={{ background: 'orange' }}>
                        <p> Some component here </p>
                    </Right>
                </Fixed>

I will implement this in my project and check if it resolves the remounting issue.

Thanks a lot for your help.

@aeagle
Copy link
Owner

aeagle commented Oct 17, 2019

@thekapadia I'll close this in view of the specific issue you've raised #37

@aeagle aeagle closed this as completed Oct 17, 2019
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