Skip to content

Conversation

nvartolomei
Copy link
Contributor

Basically the text for API docs is copied from createTransitionManager, looks like it is good, though didn't had chance to dig into router's internals.
Removed last line Returns a function that may be used to unbind the listener., as react-router manages this itself.

Let me know what you think. Hope got it right.

Fixes #3488

@taion
Copy link
Contributor

taion commented May 21, 2016

You can still unbind the listener yourself... though maybe it's not so necessary.

The comment is also not quite correct – you can return true or undefined (or actually any non-string value other than false) to not abort the transition.

@nvartolomei
Copy link
Contributor Author

@taion just amended the commits, though some rewording may be still needed. Feel free to contribute 🤗

docs/API.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not quite right – you can return

  1. A string to show a prompt
  2. false to cancel the transition
  3. Anything else (or nothing) to let the transition continue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During a normal transition, the hook function receives the next location as its only argument and can return a prompt message (string) to show the user, to make sure they want to leave the page, false, to prevent the transition. Any other return values will not have any effect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During a normal transition, the hook function receives the next location as its only argument and can return either a prompt message (string) to show the user, to make sure they want to leave the page; or false, to prevent the transition. Any other return value will have no effect.

Thanks!

@nvartolomei
Copy link
Contributor Author

nvartolomei commented May 23, 2016

@taion amended once again. Please let me know if I should update the comment for listenBeforeLeavingRoute to match the one in docs.

@taion
Copy link
Contributor

taion commented May 23, 2016

I wasn't going to make you do it, but now that you volunteered, please do. This is LGTM.

@timdorr timdorr merged commit 0616f6e into remix-run:master May 23, 2016
@nvartolomei nvartolomei deleted the docs-fix-set-route-leave-hook-references branch May 23, 2016 20:21
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants