Explicitly know it's an undo action?

[sidonaldson]

I have an interesting problem and I would love some thoughts on the best solution.

I have a map instance which has a moveend listener. After the user moves the map I trigger an action called mapMoved which stores the center and zoom etc. To prevent the map from trying to move again on componentWillReceiveProps (because I've just moved it manually) I look for the original mouseevent to know it's human interaction and add the property human:true.

Now, when I try to undo the action it works as expected except the human flag is still present. Is it an anti-pattern to suggests an optional property which flags the fact it's an undo action... e.g

{
    present:{},
    past:[],
   future:[],
   undid: true
}

Alternatively I could add some logic to stop the map moving if mapOptions.future.length === 0 (this is the simplest solution), or add logic to mapStateToProps to check the map isn't in the final position already (least ideal).

Thanks!

[davidroeca]

This looks like a complicated problem. As long as you're storing the state in redux, consider filtering actions not taken by the user so that the user can only undo actions they've taken--you'll just have to figure out how to differentiate between automatic and user-touched actions/states. If you're storing the state in the react components, this library won't know about it.

[sidonaldson]

@davidroeca agreed, this is probably a common issue with a combination of user events and integrating with non-react libraries. Preventing Redux updating the props if future.length === 0 works well and is the simplest, however I thought I would raise it in case there is better resolution that could be included in redux-undo.

[davidroeca]

Not sure I understand correctly. Does this work for an intermediary redo? e.g. UNDO, UNDO, UNDO, REDO? I'd assume future.length > 0 in this case.

The issue seems to have something to do with controlled (preferable) vs uncontrolled components, or the analog in other DOM-rendering frameworks.

The hope is that redux has two-way communication with the component. E.g. a component change yields a state change, and a state change yields a component change. This is the pattern followed by redux-form. Not every library lets you defer to external state which I think is the main challenge

[sidonaldson]

@davidroeca I'll try to clarify exactly what is happening.

As the map is moved the state is updated to create an undo point.

Then when undo is called the componentDidUpdate fires and the map then moves to the cached position - except it will call moveend after it has moved and try to set another cache point (unwanted as it resets the forward array)

There are several solutions;

• bind and unbind the moveend event as a when it's required
• set a boolean which tells the component an undo event has been triggered and unset it once it's completed

OR (this was the original feature request)

The undo event returns future, past and present but also a boolean flag which says it's an undo?

{
    present: [1],
    past: [2.3,4,5,6,7,8,9],
    future: [0],
    undo: true <--
}

or you could further add some shorthand sugar to expose the index and the limit

{
    present: [1],
    past: [2.3,4,5,6,7,8,9],
    future: [0],
    index: 1,
    limit: 10
}

It's a crazy simple idea but would have been useful so I felt it worth mentioning! Thanks for a great product.

[davidroeca]

Well it's thanks to issues such as this one that make the library better!

I think previousMove as an integer might make more sense than an undid boolean. previousMove for undo would be -1, 1 for redo, the number of moves in a jump action, or 0 for any other action.

I also like the index/limit idea--not sure which would be more useful. previousMove is probably a cleaner addition to the library right now, and is also much less easily inferred from the state history

@sidonaldson would previousMove work for your use case?

[sidonaldson]

@davidroeca yes, exactly, and that nicely supports the redo. Considering the index/limit idea you would only need one additional property called `lastIndex' or similar to complete the set. Exposing limit (if set) would still be a major advantage imo. Either solutions would be a great addition. :)

[davidroeca]

since #167 is merged, I'm closing this issue. Please reopen if there are still challenges here

[xxlee0927]

Consider the situation that setting limit to 10 and current index is 9 (i.e. the 10th step).

{
  index: 9,
  limit: 10
}

then perform undo

{
  index: 8,
  limit: 10,
}

and then redo it

{
  index: 9,
  limit: 10,
}

How do I explicitly detect the last redo action with only index and limit?