PopupDemo regression

[TomasMikula]

There is a regression in the PopupDemo (gradle PopupDemo), introduced after 0.6.10: the position of the popup does not update as I type.

[JordanMartinez]

The regression doesn't exist as of 7277f9f (the last commit before the skin was removed), but shows up somewhere after I removed the skin. When my work was merged in 45145ff, this regression appears.

I've narrowed it down to this. When user types/deletes a character, the following path occurs:

layoutChildren() // calls positionPopup()
---
positionPopup() { // calls getSelectionBoundsOnScreen()
switch(alignment.getAnchorObject()) {
        case CARET:
            bounds = getCaretBoundsOnScreen(); break;
        case SELECTION:
            bounds = getSelectionBoundsOnScreen(); break;
    }
    // at this point, bounds is null when it should have a value to which
    // popup would position itself.
    bounds.ifPresent(b -> { /* code */ })
}
---
getSelectionBoundsOnScreen() { 
// calls getCaretBoundsOnScreen 
// since selection's length is 0
    IndexRange selection = getSelection();
        if(selection.getLength() == 0) {
            return getCaretBoundsOnScreen();
        }
}
---
getCaretBoundsOnScreen() {
    // virtualFlow.getCellIfVisible(getCurrentParagraph()).isPresent() == false
    return virtualFlow.getCellIfVisible(getCurrentParagraph())
        .map(c -> c.getNode().getCaretBoundsOnScreen());
}

[JordanMartinez]

Edit: PopupDemo uses the PopupAlignment: SELECTION_BOTTOM_CENTER, so this should occur. I didn't know it at the time of this comment.

Tomas, when a user selects text from the left and going right in the demo, should the popup be (almost) directly underneath where the caret is as it progresses to the right?

When I positioned the caret somewhere, pressed and held Shift, and used the right arrow to further select characters on the right, the popup didn't line up vertically with the caret.

[JordanMartinez]

Changing getCaretBoundsOnScreen to the following solves the original issue (but not the selection one). So, is this a regression in Flowless?

Cell<Paragraph<S, PS>, ParagraphBox<S, PS>> cell = virtualFlow.getCell(getCurrentParagraph());
    if (virtualFlow.visibleCells().contains(cell)) {
        return Optional.of(cell.getNode().getCaretBoundsOnScreen())
    }

[JordanMartinez]

The code on the commit immediately after 7277f9f (where Flowless is updated to the then-current 0.5-Snapshot) works, so I guess it really is some thing caused in my code and not Flowless's new version.

[JordanMartinez]

When a user types a letter, positionPopup() (and therefore layoutChildren()) is called twice in the pre-skin-removal commits but is only called once in the post-skin-removal commits.

The necessary bounds that would update the popup position are not present in the first call but are in the second. So, I'm guessing that my code somehow prevents the second call to layoutChildren()

[JordanMartinez]

Ok, I redid a one of my skin-removal commits (I inlined StyledTextAreaView's content into StyledTextArea) in my branch "regressionCheck", along with a few other commits that later followed: popupAnchorAdjustment's naming conflict and removing a reference to view/virtualFlow in StyledTextAreaBehavior because it's no longer needed.

In this branch's second-to-last commit, there is no regression in the code. Despite comparing the differences between regressionCheck and the current master branch, I can't find major differences that would show what's causing the problem.

However, I have found that the regression does appear in regressionCheck when I move VirtualizedScrollPane outside of STA.

So, the last commit in the branch (currently 7c44fa08de37fd1fc7440b64c71bba4fe5795b78) includes the regression (which can be removed if one follows the commenting out instructions: // ### comment out instructions ###

[JordanMartinez]

Wrapping positionPopup() in Platform.runLater() fixes the problem, but I'm not sure if that's the cleanest way to fix this regression. See my PR #238

[JordanMartinez]

Turns out this was a deficiency in Flowless. See #238 and FXMisc/Flowless#19

[TomasMikula]

Fixed by FXMisc/Flowless#20