FXMisc / UndoFX

Undo manager for JavaFX
BSD 2-Clause "Simplified" License
99 stars 17 forks source link

UndoManager#forgetHistory should invalidate undoAvailable, redoAvailable, etc. #1

Closed ryanjaeb closed 10 years ago

ryanjaeb commented 10 years ago

If forgetHistory discards all previous history, should it invalidate the undoAvailable and redoAvailable properties? Also, should it automatically update the marked position to ensure it isn't pointing to some old, forgotten position?

TomasMikula commented 10 years ago

Good catch, undoAvailable should be invalidated on forgetHistory.

redoAvailable is not affected by forgetHistory.

Not updating the marked position is by design. Applications can use marking to mark a position in history when the document was saved. By forgetting history, the document does not become saved.

benedictleejh commented 8 years ago

Sorry to bump an old issue, but if I need to clear the redo, how should I do it then? Since forgetHistory doesn't affect redo.

TomasMikula commented 8 years ago

Hmm, well, I didn't think of that. What is the use case for it?

benedictleejh commented 8 years ago

On creating a new document, changes in the redo queue should not be able to be applied to the new document, but as it is currently, you can, with the caveat of potentially strange behaviour depending on the program.

TomasMikula commented 8 years ago

What about creating a new UndoManager for the new document? On Oct 29, 2015 1:27 PM, "Benedict Lee" notifications@github.com wrote:

On creating a new document, changes in the redo queue should not be able to be applied to the new document, but as it is currently, you can, with the caveat of potentially strange behaviour depending on the program.

— Reply to this email directly or view it on GitHub https://github.com/TomasMikula/UndoFX/issues/1#issuecomment-152257217.

benedictleejh commented 8 years ago

Hmm, I tried your suggestion, but while the actual queue is cleared, bindings to redoAvailableProperty aren't, so on a UI level, that shows up and could be confusing.

TomasMikula commented 8 years ago

Well, you need to rebind it to the new UndoManager's redoAvailableProperty. You might want to use flatMap so that you don't have to rebind "manually":

class Document {
    UndoManager getUndoManager();
}

Val<Document> currentDocumentProperty = ...;
Val<Boolean> redoAvailable = currentDocumentProperty
        .flatMap(doc -> doc.getUndoManager().redoAvailableProperty())
        .orElseConst(false);
redoButton.disableProperty().bind(redoAvailable.map(x -> !x));

Now as you change the Document in currentDocumentProperty, the redoButton will automatically switch to its UndoManager's redoAvailableProperty.

benedictleejh commented 8 years ago

Hmm, I'll need to meditate on that for a while. My application currently uses a global undo manager, and I expose undoAvailableProperty and redoAvailableProperty as

def undoAvailable: ObservableBooleanValue = undoManager.undoAvailableProperty()
def redoAvailable: ObservableBooleanValue = undoManager.redoAvailableProperty()

and fire events using their onChange methods (from ScalaFX)

 UndoController.undoAvailable onChange { (_, _, available) =>
    EventBus.send(UndoStatus(available, UndoEventSourceIdentity))
  }
  UndoController.redoAvailable onChange { (_, _, available) =>
    EventBus.send(RedoStatus(available, UndoEventSourceIdentity))
  }

What you're describing could work, but I need to figure out how to make it work in context.

TomasMikula commented 8 years ago

Just replace the global UndoManager with one that comes with the document. It doesn't really have to be part of the document; you can have e.g. a DocumentWrapper that holds a document and an UndoManager and then have a Property<DocumentWrapper>. Then flatMap that property to define redoAvailable, undoAvailable.

benedictleejh commented 8 years ago

Ok, after meditating over it for a while, how I did it in the end was to wrap the UndoManager inside an ObjectProperty and using EasyBind to flatMap the ObjectProperty to expose the required properties.

TomasMikula commented 8 years ago

:+1: