davidmfoley / react-router-modal

Simple modals for react-router 4
MIT License
153 stars 20 forks source link

closeModal and BackdropClick should not add a new entry to the history #14

Open Unruly-Coder opened 6 years ago

Unruly-Coder commented 6 years ago

closeModal and BackdropClick should behave exactly the same as clicking back button on a browser. Adding a new history entry is very confusing for the user especially on Android mobile devices where the native back button is used extensively. I don't see any use case where adding the new history entry on close a modal is useful.

davidmfoley commented 6 years ago

This is a good feature request but note that emulating the back button does not work in all cases -- it depends on the context. For example: one can link to a modal route like /customers/:customerId/orders/:orderId from a different route like /admin/orders.

Also, a route with a modal can exist with an empty history: either the user refreshes the page or the user is brought to the page from an email link or a _target='blank' anchor.

This may be as simple as testing whether the previous history entry is a parent of the modal route path and conditionally using history.replace(...) or history.goBack().

zomars commented 6 years ago

I'm using this modal to select a value for a form, but I lose the selected data when manipulating history. I don't know if there is a way to close the modal without losing data.

davidmfoley commented 6 years ago

@zomars If you share a snippet, I can try to help you.

axedre commented 6 years ago

Agreed - parentPath should accept either a string representing the state the app should transition to when the modal is closed (i.e. the current behavior) or a special value, e.g. '#back' that does history.goBack(). Any chance of seeing this feature implemented?

davidmfoley commented 6 years ago

@axedre I think this should be configurable by a closeBehavior prop on the ModalContainer with support to override on ModalRoute.

I'm not sure if a set of strategies that are tried in order, specified by strings like ( go-back, go-back-if-previous-path-is-parent, replace-path, push-path ) makes the most sense, or an onRequestClose function where you can do whatever you want.

Then the default behavior can be the current behavior (push) and one can opt in to the new behavior.

What do you think?