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

Component state is not persisted #31

Closed
z-vr opened this issue Jun 7, 2017 · 3 comments
Closed

Component state is not persisted #31

z-vr opened this issue Jun 7, 2017 · 3 comments

Comments

@z-vr
Copy link
Contributor

z-vr commented Jun 7, 2017

The component state is not persisted when opening a new popup, then coming back to the old one.
Checkout the demo - open a new popup, then use a button inside the popup to create a new one.

popup
constructor 1
component 1 will mount
popup
component 1 will unmount
constructor2
component 2 will mount

Now if you close the second popup, you will see:

component 2 will unmount
constructor 1
component 1 will mount

what we don't really want is constructor 1 happening again, because we want to keep the state of the first component.

I think when you do

    onClose() {
        this.setState(initialState); // set content to null
    }

This destroys

                    <div className={this.className('box__body')}>
                        {this.state.content}
                    </div>

and also probably when switching between contents, react is not able to restore them afterwards. A solution would be to keep all contents in a popup dom tree hidden, and only show the relevant one? Can't think of anything else. As a workaround, I have to pass state in a mutable object when opening popup.

Here's the source code

Have you got any thoughts on this?

@tbleckert
Copy link
Contributor

Ah, hm. Didn't think about that scenario. That complicates stuff for sure. I need to give this some thought. I think some sort of navigation wrapper would be the best in these sorts of scenarios. Instead of multiple popups you have one popup that can hold multiple views.

class ContentNavigation extends React.Component {

    constructor(props) {
        super(props);

        this.state = {
            views: this.props.views,
            currentView: 0
        };
    }

    render() {
        return this.state.views[this.state.currentView];
    }

}

Then you would pass this as the content to the popup and either use buttons inside this component or set it up so your popup buttons can navigate.

Seems more natural to have it like this. If for example you're displaying a form with multiple steps inside the popup it feels more "correct" (in lack of a better word) to have a form component that manages the steps.

What do you think? This could be made into a plugin and used like this:

Popup.plugins().multipleViews('The title', [/* Array of components/views */], {/* object of labels for buttons: next, back, cancel, complete */});

@z-vr
Copy link
Contributor Author

z-vr commented Jun 7, 2017

@tbleckert yeah it definitely makes things trickier. I like the views idea, but in my particular case I don't have step-by-step form completion, I open popups from within a form popup, e.g.,

LOCATION

Location: [_______]
Candidate: [_______]
   (add candidate)
Office Address: [_______]
   (add address)

The views are dynamic here, and it would be better to have an internal mechanism dealing with it, when pushed / popped from the queue.

It is true that a form can have a component that manages steps, but I don't think it would be 100% relevant to the popup function, since it's very case-specific.

@tbleckert
Copy link
Contributor

I've thought about this but can't really come up with a good way to handle this. And I'm not sure if this is something that should be handled inside the component.

The best solution, imo, is to manage the state in your application. For example, you can have a component that saves all changes through Redux and always re-apply it when mounted.

What do you think about that? I'm closing but feel free to comment and/or reopen.

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