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

Make MathKeyboard IDisposable #179

Merged
merged 2 commits into from
Oct 24, 2020
Merged

Conversation

charlesroddie
Copy link
Collaborator

fixes: #178

NuGet.Config Outdated Show resolved Hide resolved
@Happypig375
Copy link
Collaborator

My Visual Studio does not do this. I am confused as to why does this happen.

@codecov-io
Copy link

Codecov Report

Merging #179 into master will decrease coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
- Coverage   87.33%   87.29%   -0.04%     
==========================================
  Files         156      156              
  Lines       11312    11314       +2     
==========================================
- Hits         9879     9877       -2     
- Misses       1433     1437       +4     
Impacted Files Coverage Δ
CSharpMath.Editor/MathKeyboard.cs 88.93% <0.00%> (-0.38%) ⬇️
CSharpMath/Atom/MathList.cs 86.20% <0.00%> (-2.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed72190...7697c28. Read the comment docs.

@charlesroddie charlesroddie merged commit 476beb0 into master Oct 24, 2020
charlesroddie added a commit that referenced this pull request Oct 27, 2020
* Add setting LaTeXSettings.PlaceholderBlinks + unit tests

* Fix PlaceholderBlinks setting + add unit test CaretStillBlinks

* Non-blinking placeholder test "CaretStillBlinks" should also verify that the RestingNucleus is shown for all placeholders if the caret blinks

* Refactor: if PlaceholderBlinks is false and ShownThroughPlaceholder true then don't change the CaretState instead of ignoring the changed state later on

* Remove MathKeyboardCaretState.ShownThroughPlaceholder
and avoid invoking RedrawRequested when state has not changed

* Remove nuget.config

* add xml

* Set PlaceholderBlinks default to false

* Don't show cursor if at placeholder

* ternary conditional operator

* fix ProcessCaretState

* Refactor away ProcessCaretState() + fix indentation

Also:
- Set PlaceholderBlinks back to true because I don't like to do a commit that has failing unit tests. A commit that changes a default, should also change the tests. I will have a look at the changes needed.

* change LaTeXSettings.PlaceholderBlinks again and remove tests checking for default blinking behaviour. Remove nuget.Config

* Restore parts of some tests + mark CaretIsOverriddenByPlaceholder as "fix or delete"

* Replace CaretState enum property by boolean properties "InsertionPositionHighlighted" and "ShouldDrawCaret"

General notes:
Having a CaretState that says "MathKeyboardCaretState.Shown" while actually no caret is shown because a placeholder is shown is wrong. Having a CaretState "MathKeyboardCaretState.Hidden" and "MathKeyboardCaretState.TemporarilyHidden" is not needed: you can use StopBlinking() just after setting the CaretState you want to keep until the next key press. These two observations resulted in the boolean properties "InsertionPositionHighlighted" (that makes sense for both the caret AND the placeholder appearance) and "ShouldDrawCaret".

Because Drawing the caret is done in CSharpMath.Rendering.FrontEnd, the unit tests of CSharpMath.Editor can only test "ShouldDraw" and unit tests that do that can cover the same as before (when it was tested via a MathKeyboardCaretState enum).

Notes about moved unit tests:
- CaretIsOverriddenByPlaceholder has been replaced by PlaceholderDoesNotBlinkAndNoCaretVisible.
- CaretMovesWithPlaceholder has been replaced by NonBlinkingActivePlaceholderMoves.

* Make mergable without conflict (after #179 for IDisposable MathKeyboard)

* Move method before first using

* Fix inconsistent code style

* Reverse assertion order

* Reverse assertion order - part 2

Co-authored-by: Charles Roddie <[email protected]>
Co-authored-by: FoggyFinder <[email protected]>
@Happypig375 Happypig375 added Resolution/Implemented The described enhancement or housekeeping work has been implemented. Type/Housekeeping This includes internal only changes. labels Oct 27, 2020
@charlesroddie
Copy link
Collaborator Author

@Happypig375 could you queue a nuget release with this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution/Implemented The described enhancement or housekeeping work has been implemented. Type/Housekeeping This includes internal only changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The IDisposable blinkTimer in MathKeyboard is not handled
4 participants