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

Use customHistory url instead of internal url in render if available #140

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hesselbom
Copy link

@hesselbom hesselbom commented Feb 7, 2017

Currently, when using 'history' module as customHistory, and blocking the history change, the internal state url in Router is still updated, even if window location isn't. This pull request aims to fix this.

@developit
Copy link
Member

@hesselbom how are you blocking the history change? Just trying to set up a demo.

@hesselbom
Copy link
Author

hesselbom commented Apr 6, 2017

/** @jsx h */
import { h, render } from 'preact'
import { Router, Link } from 'preact-router'
import createHistory from 'history/createBrowserHistory'

let history = createHistory()
history.block('Are you sure you want to leave this page?')

let ViewOne = () => <Link href='/two'>Go to view two</Link>
let ViewTwo = () => <Link href='/'>Go to view one</Link>

render((
  <Router history={history}>
    <ViewTwo path='/two' />
    <ViewOne default />
  </Router>
), document.body)

When trying to switch view but canceling in the prompt the view will update but the url won't.

@developit
Copy link
Member

oh wow, I didn't know history.block was even a thing!

@hesselbom
Copy link
Author

Neither did I until I conveniently found it when needed for a very specific use case! ;)

@developit developit self-requested a review April 7, 2017 13:41
@developit
Copy link
Member

Just circling back to this - shouldn't this handler be getting fired to take block() into account?
https://github.com/developit/preact-router/pull/140/files#diff-1fdf421c05c1140f6d71444ea2b27638R193

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

Successfully merging this pull request may close these issues.

None yet

2 participants