Skip to content

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

Open
hesselbom wants to merge 1 commit intopreactjs:mainfrom
hesselbom:master
Open

Use customHistory url instead of internal url in render if available#140
hesselbom wants to merge 1 commit intopreactjs:mainfrom
hesselbom:master

Conversation

@hesselbom
Copy link
Copy Markdown

@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
Copy Markdown
Member

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

@hesselbom
Copy link
Copy Markdown
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
Copy Markdown
Member

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

@hesselbom
Copy link
Copy Markdown
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
Copy Markdown
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.

2 participants