[RFC] New TextBox style

[nicolaihenriksen]

This draft PR is intended as a place for discussing the approach I am currently taking for the rework of the styles using SmartHint (TextBox being the first one). Sorry about the long write-up! Make sure you have a couple of Mountain Dews at hand before you dive in 😄

Scope

I think the scope of this discussion should not deal with the SmartHint as such, but mainly focus on the layout of the TextBox style.

What am I trying to solve/address

There have been a number of issues opened with elements inside the TextBox not aligning nicely (vertically). The current implementation uses a bunch of converters with "magic numbers" applied in order to attempt to align elements nicely for all/most scenarios. The new style should aim to get rid of all/most of these converters, and ideally not use any magic numbers.

Basic idea

Capture all the elements that should align with each other (vertically) in a single grid.

The blue box in the image above represents the single grid encapsulating all the elements of the TextBox that need to align vertically with each other. The red boxes marks each of the individual elements. The challenge is that these elements, in various ways, can have their size individually controlled from the calling code. So the basic idea is to simply have VerticalAlignment=Center on all the elements with the red boxes, so they vertically align to the middle of the blue box regardless of their individual sizes. The blue box is the element that respects the TextBox.VerticalContentAlignment property so that it can align top/center/bottom. This works well for all variations of VerticalContentAlignment with the Stretch case, when the TextBox is also multi-line, being the exception. This is an edge case that I think we need to discuss, because I know people have different opinions on what they expect in this scenario.

The edge case

The edge case in question is, as mentioned, when the TextBox is multi-line (i.e. AcceptsReturn=True) and VerticalContentAlignment=Stretch. In this case the element containing the text that the user enters is expected to align to the top but be allowed to stretch downwards to fill up the space. In order to do this, the blue box would then have VerticalAlignment=Stretch applied to it, and ,as a minimum, the red box with the user content should also have VerticalAlignment=Stretch to fill the space vertically. But what about the other elements? Should they also top align? And if so, they should probably (vertically) align nicely with the first row of text entered by the user? While this may seem like a good approach at first glance, it comes with a lot of issues to deal with. What if the icons/buttons have been set with non-default sizes, or the FontSize has been increased/decreased? Aligning such that those elements vertically center around the first row of text can become a complex calculation per element (i.e. red box). If calculations are not done, the result could look something like this (fixed height set on the TextBox in this example, but that is irrelevant) where you can see there really is no vertical alignment between the elements from the red boxes:

Personally I don't like this. If we were to align the elements vertically around the first row of text, I suspect we would end up with a bunch of complex converters, and possibly even some negative margins and ClipToBounds=False just to achieve what we want.

My suggestion is to always keep all other "red box" elements, apart from the actual user content (and possibly prefix-/suffix-texts) at VerticalAlignment=Center inside of the blue box. This would give a result similar to this.

Personally I don't think the prefix-/suffix-text should top-align in this scenario, but since they are TextBlock elements which inherit the same FontSize, making those align nicely with the first row of text would probably be doable (or even come for free).

So what am I hoping to achieve with this RFC?

I hope that I have provided enough details above, along with the code in the branch, to give the reader an understanding of how I intend to structure the new TextBox styles. I am looking for comments/feedback/pushback/critique; all is welcome.

This is just a first step, because the second issue that I want to address, is actually the placement of the SmartHint. This is also done using a bunch of converter magic with magic numbers and a lot of bindings making it complex to understand what is actually happening. But I doubt that one thing can be fixed without also making changes to the other, so I will most likely end up making a PR with changes for all the styles that use the SmartHint 😨 In that regard, should such a PR do the same as I am currently doing in this PR and create a parallel implementation rather than modifying the existing one? That definitely makes it easier to compare them, but is probably not ideal to actually push to master branch. Ideas?

If you run the code from the branch, you will open the demo app on the "Smart Hint" page where I have added the new TextBox style at the top for comparison with the default ones below it.