2+ Views (areas) share the same Model (document)

[JordanMartinez]

This is the same result of the #231 but with a cleaner commit history.

[TomasMikula]

I would probably just call it getDocument().

[JordanMartinez]

I would but there's already a method with that name (see Line 336).

[TomasMikula]

Right. What about getContent()?

[JordanMartinez]

Sounds good. I've updated the method name.

[TomasMikula]

addSuspendable, removeSuspendable look a lot like adding/removing special kind of listeners. This suggests a question whether we can reuse some existing listener mechanism. We can! We can have a SuspendableNo beingUpdatedProperty() on EditableStyledDocument (in addition to the ones on StyledTextAreas) and add a listener that suspends the StyledTextArea's omniSuspendable. I added Suspendable.suspendWhen(condition) to ReactFX to make it easier, so that one can now do something like

manageSubscription(omniSuspendable.suspendWhen(content.beingUpdatedProperty()))

[JordanMartinez]

Aye... that would be much cleaner. Did you make a new release so I can update the code?

[TomasMikula]

Yes, I updated the snapshot release (2.0-SNAPSHOT).

[JordanMartinez]

Great. I've had my first run-through it. If EditableStyledDocument has a beingUpdatedProperty() now, should I remove the StyledTextArea's beingUpdated SuspendableNo ? (I know we ported it to ESD in the previous PR's exploration, but I reported it back to STA because it didn't add any value at that time.)

[JordanMartinez]

I ported beingUpdated to ESD. How's that?

[TomasMikula]

Great. I've had my first run-through it. If EditableStyledDocument has a beingUpdatedProperty() now, should I remove the StyledTextArea's beingUpdated SuspendableNo ? (I know we ported it to ESD in the previous PR's exploration, but I reported it back to STA because it didn't add any value at that time.)

No, we need to keep another beingUpdated in STA, in order to guarantee the property that no change can be observed while beingUpdated is false. This way, the user can be sure that when beingUpdated changes from true to false, all other changes have been propagated and are thus consistent with each other (e.g. caret position, current paragraph, length, ...). Note that the order in which registered listeners are invoked is undefined (neither in JavaFX nor ReactFX). If STA observes the same beingUpdated as the user, the user could get the notification first, before STA releases the pending notifications.

[JordanMartinez]

Ah... I see. Fixed it.

[TomasMikula]

👍

[JordanMartinez]

Cool 😃

[TomasMikula]

So I merged this, but the supported use is limited. For example, if the document is edited from both areas, undo will not work very well. One area's undo will be regarded as a new edit in the other area. It is a complex problem on its own, so for now I would just say that such a use is not supported.

[JordanMartinez]

Dang.... Yeah, that's quite a problem.

On another note, when will there be a new mainstream release (meaning, not a snapshot release)?

[JordanMartinez]

if the document is edited from both areas, undo will not work very well. One area's undo will be regarded as a new edit in the other area. It is a complex problem on its own, so for now I would just say that such a use is not supported.

I'll open an issue for this as well, just so it's on file.

[TomasMikula]

I can make a new release as soon as

1. all known regressions introduced by these refactorings are resolved.
2. all SNAPSHOT-versioned dependencies are released :)

[JordanMartinez]

all known regressions introduced by these refactorings are resolved.

What are the known regressions? Would this include the Undo-related problem mentioned above?

all SNAPSHOT-versioned dependencies are released :)

:) What else needs to be done in Flowless and ReactFX before they get their releases?

[TomasMikula]

What are the known regressions?

#210, #220, #234

Would this include the Undo-related problem mentioned above?

That's not really a regression, since that was not supported at all before.

What else needs to be done in Flowless and ReactFX before they get their releases?

They might be good to go.

[JordanMartinez]

I've also noticed a problem in the richtextfx's build.gradle file. It lists UndoFX as a dependency twice: the first, with its transitive dependencies overrules the second, one without transitive dependencies. I've had to comment out the first instance or else I get compile warnings.

[TomasMikula]

Yeah, the first one should have been deleted when the second one was added. This is a mistake. Anyway, the transitive = false options will be removed when all dependencies use the same version of ReactFX as RichTextFX.