Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move UndoManager creation code outside StyledTextArea #333

Closed
JordanMartinez opened this issue Jun 21, 2016 · 2 comments
Closed

Move UndoManager creation code outside StyledTextArea #333

JordanMartinez opened this issue Jun 21, 2016 · 2 comments

Comments

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Jun 21, 2016

Relates to #233. In order to have non-linear undo/redo capability, StyledTextArea and its model class will need to take an UndoManager argument and push the creation code outside of the area itself.

There's a few things to keep in mind:

  1. Since the area (which IMO adheres more to the MVVM paradigm now) could be used in a larger view component, the definition of a valid change can now be different than the normal ones given. Opening up this argument would give more flexibility to the developer to determine what a "change" actually is.
  2. With a non-linear undo/redo capabilities, the size of the queue can change between clones. One might have an unlimited queue, another could have a fixed size queue, and a third a zero-size queue. As such, an UndoManagerFactory can no longer be the main argument passed because it doesn't give the developer the freedom to choose which queue size to use. One exception to this is to pass in a QuadFunction that accepts the arguments now found in StyledTextAreaModel#createRich/PlainUndoManager. However, this might restrict the developer due to the first point made.
  3. Since an EditableStyledDocument exists outside of the area before being inserted into one then that document could, at least technically, be modified by a non-StyledTextArea class. If that occurs, the code should be robust enough to update all the areas displaying its content to account for that change (e.g. a valid undo might no longer be valid). This implies that the ``UndoManagerFactoryshould somehow be tied to theEditableStyledDocument` for the lifetime of that document.
    • I haven't been able to think of a good way to tie the two items together. Perhaps some object that groups the two together could work, but how does the UndoManagerFactory know which items to ignore (an area changed the document) and which to accept (the document was modified directly)?
    • At the same time, this possible bug exists even right now: an area's UndoManager's next undo/redo could be invalidated if a developer edited that document directly instead of through the area. Rather than accounting for that possibility (why would that happen in the first place?), it just shouldn't be done.
  4. A new filtering mechanism is needed to filter which events an area did and which it did not do to insure that that area's UndoManager's next undo/redo is only the one that the area did and not some other clone's change. UndoFX already uses something like this to ignore changes to the content that it performs:
private final EditableStyledDocument<?, ?> document = // creation code
private final SuspendableNo performingUpdate = new SuspendableNo();

public final EventStream<TextChange> textChanges() {
    return document.textChanges().conditionOn(performingUpdate);
}

performingUpdate.suspendWhile(() -> replaceText(0, 0, "Some text"));
@JordanMartinez
Copy link
Contributor Author

By including an UndoManager from UndoFX as one of the parameters in the StyledTextArea, it forces the developer to use UndoFX for their undo/redo system. This does not give the user "complete control" (Tomas' words) over how undo/redo handling works. Rather, an UndoManager, whether from UndoFX or custom-made, should be something that can be implemented on top of the area by the developer.

Thus, RTFX should provide a factory method that can install a basic plain or rich UndoFX UndoManager into an area for convenience.

@JordanMartinez
Copy link
Contributor Author

Closed by #528.

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

No branches or pull requests

1 participant