Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add support for Surrogate Unicode sequences #139
Add support for Surrogate Unicode sequences #139
Changes from 9 commits
f087bf5
9446c9b
637747c
559fe7b
ef299e5
ebc751d
e0a2a52
352bb67
adeb5ea
c174f9b
479460b
cc417f8
7e36247
88ebb08
5891381
e4e247b
532ac52
b7c1e35
8cab697
78710e5
2be4221
265eb64
8c76411
0b478fc
9791ab4
6c008eb
272b00f
d4fc7f1
019a80f
a823630
3a5efde
61b9697
40dedcc
379870e
a829699
51b30fe
7560a6c
b97e7bc
c356590
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With "clearing old caret" approach, we do not detect whether more than one carets were requested. If an attempt happened to draw one more caret, then only second will be "painted". We should keep checking that only caret can be drawn at a moment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I did this is there was a check and exception thrown on the second caret found. I assumed this meant your intent was for only a single caret.
I can simply remove the check... But what is the scenario for two carets? Can that happen in avalonia?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I tried not toggling and removing the check and what happens is that it completely broke cursor navigation on pages with multiple input controls.
I think way I have it is correct, or at least if there is a scenario where we need this it can be a future PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opposite, I'm saying that we should throw an exception (may be optional tho) when two carets have been tried to show.
it should not be legit to try to show two cursors, we can not determine which one is supposed to be correct.
Any control is supposed to show the caret only when focused. Avalonia tracks the focus: any focus change starts with LostFocus in old place and only then GotFocus in new place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to reproduce it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomlm we can move this discussion to a separate topic to merge this PR if you think would be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It confuses me only because I expected there are no attempts to show double cursor which we just can not do anyway. If this attempt happens, I would better investigate why and fix that instead of allowing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, I will dig into why this is happening now. It must be something about my changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you push intermediate changes with the repro, I could try to investigate in parallel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found it, you are right.
I introduced a bug in Pixel.Blend(). I was merging IsCaret when merging regular foreground pixels by falling through to the the isCaret section instead of returning the blended pixel. I have fixed it, and the exception doesn't throw anymore.
Good catch!