ZacharyRSmith / react-router-navigation-prompt

A replacement component for the react-router `Prompt`. Allows for more flexible dialogs.
MIT License
190 stars 36 forks source link

(HashHistory not supported) Changing URL in browser and clicking confirm doesn't navigate #36

Open mcupito opened 6 years ago

mcupito commented 6 years ago

When I change the URL directly in the browser, I am seeing that my page is not actually navigating upon confirm of the action. I've briefly debugged and saw that the _state.action is POP when this occurs -- unsure if I can manually try to make this PUSH and see what happens.

Scenario:

On test.com/x Type in test.com/y Pop-up appears Click Yes, Leave the Page (onConfirm) Prompt closes and nothing else happens, but the URL stays the same

I am using React with Redux, along with React Router.

Here's the implementation :

import ConfirmModal from "../Confirm/Modal";

class NavigationBlocker extends React.Component {
    constructor(props) {
        super(props)
    }

    render() {
        return (
                <NavigationPrompt when={this.props.canSave}>
                    {({ onCancel, onConfirm }) => (
                        <ConfirmModal
                            titleText={this.props.title}
                            onCancelClicked={onCancel}
                            onActionClicked={onConfirm}
                            visible={true}
                            cancelLabel={'No, keep working'} 
                            actionLabel={'Yes, leave anyway'}
                        >
                            <p>{'Your changes were not saved. Do you still want to leave?'}</p>
                        </ConfirmModal>
                    )}
                </NavigationPrompt>
        );
    }
}

The ConfirmModal has a decent amount of code in it, but it doesn't truly do anything except render what was passed in and call the props as actions :

<Button type="button"  onClick={this.testActionClick} >
    <strong>{this.props.actionLabel || 'OK'}</strong>
</Button>

<a type="button"  onClick={(e) => this.cancelClicked(e, this.props.onCancelClicked)}>
    <strong>{this.props.cancelLabel || 'Cancel'}</strong>
</a>

Sidenote: I did not implement this code, but noticed this behavior and have since taken over trying to resolve it. I am aware that this implementation doesn't use any fancy comparison of location or anything that doesn't ship out of the box, so it could be a lacking implementation.

Thanks for your time!

chiumax commented 6 years ago

Hmm, this issue doesn't happen with any of my projects. Try looking at this example code that works: https://codesandbox.io/s/oq5rrjqjx9 Maybe it might help

ZacharyRSmith commented 6 years ago

Thanks for the issue, @mcupito .

When you say "Prompt closes and nothing else happens, but the URL stays the same", do you mean that the URL stays at test.com/x and does not navigate to test.com/y?

ZacharyRSmith commented 6 years ago

Also, it looks like you're passing NavigationPrompt's onConfirm as ConfirmModal's onActionClicked and calling ConfirmModal's this.testActionClick. Would you please share the method this.testActionClick?

mcupito commented 6 years ago

Yes for your first question, @ZacharyRSmith. Also, the testActionClick was just a placeholder for me to put a debugger.

What I end up seeing occur is that the URL changes to test.com/y in the browser, but the page never navigates and an error occurs within react-router (it appears) :

browser.js:49 Warning: Hash history cannot PUSH the same path; a new entry will not be added to the history stack

ZacharyRSmith commented 6 years ago

@mcupito are you using react-router's HashHistory instead of BrowserHistory? unfortunately, NavigationPrompt is not compatible with HashHistory (i think...i haven't tested it). it seems someone else had this issue with HashHistory: https://github.com/ZacharyRSmith/react-router-navigation-prompt/issues/2#issuecomment-358891377

btw, if you look at the code, NavigationPrompt will push or replace, never pop: https://github.com/ZacharyRSmith/react-router-navigation-prompt/blob/master/src/index.js#L115

if you'd like to Pull Request a change that would make NavigationPrompt compatible with HashHistory, that'd be awesome!

mcupito commented 6 years ago

@ZacharyRSmith Yes we are using Hash -- As far as a PR, let me [find the time to] dig in a little deeper and see if I could provide a reasonable solution. Thanks for your responsiveness and your time!

007arunwilson commented 6 years ago

browser.js:49 Warning: Hash history cannot PUSH the same path; a new entry will not be added to the history stack Same Issue for me in HashRouter

riteshahlawat commented 2 years ago

When implementing my own router prompt (without this package), I solved this issue by using history.replace() instead. Doing so when using hash history may be worth looking into.

ZacharyRSmith commented 2 years ago

Thanks, @riteshahlawat ! If anyone reading this would like to open a PR that uses history.replace() when using HashHistory, I think I'd merge. Cheers!