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

Added background color to spell checker demo. Fixed order of shapes s… #408

Merged
merged 1 commit into from
Dec 24, 2016

Conversation

afester
Copy link
Collaborator

@afester afester commented Dec 21, 2016

Fix for issue #407:
Added background color to spell checker demo.
Fixed order of shapes so that underlines are shown even if a background shape is used.

…o that underlines are shown even if a background shape is used.
@JFormDesigner
Copy link
Contributor

@afester the fix has the side effect that the underlines are now painted over the text (e.g. if text contains "gyqp" characters)

I think it would better to use:

getChildren().add(1, underlineShape);

Then the underlines are painted over the background, but under the text.

@afester
Copy link
Collaborator Author

afester commented Dec 21, 2016

@JFormDesigner I know - but is that not the expected behavior? For example, Eclipse paints the spell checking line over the text:

image

and MS Word shows the same behavior.

Anyway, getChildren().add(1, underlineShape); would work - the background nodes and the underline nodes for each segment would then be inserted alternately at the beginning of the ParagraphBox (followed by the selection and the caret shape, followed by the segment nodes). (Note: another effect is then that the underline is hidden by a text selection).

@JFormDesigner
Copy link
Contributor

@afester OK, sorry, you're right. It's better to have the underlines paint over all other, especially the text selection.

@TomasMikula
Copy link
Member

👍

@TomasMikula TomasMikula merged commit 41b7919 into FXMisc:master Dec 24, 2016
@afester afester deleted the Issue407 branch February 22, 2017 11:20
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

Successfully merging this pull request may close these issues.

3 participants