Skip to content

Conversation

@FoggyFinder
Copy link
Collaborator

Will be closed: #71

A new behaviour:

gif

What do you think about adding a bool property to control the behaviour?
Just in case if someone prefer seeing a caret. It will also reduce count of breaking change.

Draft until getting feedbacks.
Also: thinking about appropriate test.

@charlesroddie
Copy link
Collaborator

charlesroddie commented Nov 2, 2019

What do you think about adding a bool property to control the behaviour? Just in case if someone prefer seeing a caret. It will also reduce count of breaking change.

It should definitely not be caret + filled placeholder as it was before. That duplicates information.

Filled placeholder with no caret (which this PR implements) is the best option IMO and an unambiguous improvement. This is what Mathematica and the MS Office equation editor do.

Caret + blank placeholder could also be an option. This is what MathQuill does by default. I think this is inferior as it doesn't show the structure as well and is also a larger change to this library. So I don't think this should be an option, and so I don't think we need any options.

@Happypig375 do you agree?

@Happypig375
Copy link
Collaborator

@charlesroddie
Agreed

@FoggyFinder
The major component of CSharpMath's version is still 0. This means that we can still take breaking changes.

@Happypig375
Copy link
Collaborator

Just that the PR that changes the target to .NET Standard 2.0 is a bigger breaking change compared to others.

@Happypig375
Copy link
Collaborator

Also: thinking about appropriate test.

Should we introduce graphical tests? i.e. Comparison of rendered images, which can be done with CSharpMath.SkiaSharp.

@FoggyFinder FoggyFinder marked this pull request as ready for review November 2, 2019 19:35
@FoggyFinder
Copy link
Collaborator Author

Great then.

Should we introduce graphical tests? i.e. Comparison of rendered images, which can be done with CSharpMath.SkiaSharp.

It would be awesome. But I have never written graphical tests.
All UiTests that I wrote was for WPF apps.

@hflexgrig
Copy link
Contributor

You don't mind, if i try to make blinking cursor there and placeholder with dashed border ?

@FoggyFinder
Copy link
Collaborator Author

You don't mind, if i try to make blinking cursor there and placeholder with dashed border ?

I would suggest creating a separate issue and PR.
Small parts are really easier to review.

@Happypig375 Happypig375 added the Resolution/Implemented The described enhancement or housekeeping work has been implemented. label Nov 3, 2019
@Happypig375 Happypig375 merged commit 4242ce4 into verybadcat:master Nov 3, 2019
@FoggyFinder FoggyFinder deleted the UseActivePlaceholderOverDefaultCaret branch November 3, 2019 09:31
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/Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hide caret when placeholder is in active state (black square)

4 participants