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

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

Open
mcupito opened this issue Sep 10, 2018 · 9 comments

Comments

@mcupito
Copy link

mcupito commented Sep 10, 2018

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
Copy link
Contributor

chiumax commented Sep 10, 2018

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

mcupito commented Sep 11, 2018

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
Copy link
Owner

@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: #2 (comment)

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
Copy link
Author

mcupito commented Sep 12, 2018

@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!

@ZacharyRSmith ZacharyRSmith changed the title Changing URL in browser and clicking confirm doesn't navigate (HashHistory not supported) Changing URL in browser and clicking confirm doesn't navigate Sep 15, 2018
@007arunwilson
Copy link

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
Copy link

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
Copy link
Owner

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants