Text field character counter

[aubiyko]

I have added character counter to text field.
It is visible when both MaxLength is non-zero and (new) TextFieldAssist.ShowCharacterCounter is true. Only TextBox is supported because calculating RichTextBox's length is non-trivial and I don't think PasswordBox needs one.
I implement this via cloning (thus duplicating) styles: there is no Text property on TextBoxBase.

[jespersh]

Got a screenshot of your proposed change?

I think you could avoid duplicating the styles with visibility triggers and probably avoid the error all together by just limiting what can be written in the Textbox's MaxLength.
Spec seems to suggest this addition here: https://material.io/components/text-fields/

And personally I'd love a "Only visible when focused" counter

[aubiyko]

Got a screenshot of your proposed change?

Pardon me for forgetting the screenshot:

And personally I'd love a "Only visible when focused" counter

Good suggestion, will do.

[aubiyko]

I think you could avoid duplicating the styles with visibility triggers and probably avoid the error all together by just limiting what can be written in the Textbox's MaxLength.

The problem is RichTextBox's style's based on MaterialDesignTextBoxBase, but it doesn't have MaxLength property.
So if I apply my current implementation to MaterialDesignTextBoxBase style, MD RichTextBox's style will produce
System.Windows.Data Warning: 40 : BindingExpression path error: 'Text' property not found on 'object' ''RichTextBox' (Name='')'. BindingExpression:Path=Text.Length; DataItem='RichTextBox' (Name=''); target element is 'TextBlock' (Name=''); target property is 'Text' (type 'String')
System.Windows.DataWarning: 40 : BindingExpression path error: 'MaxLength' property not found on 'object' ''RichTextBox' (Name='')'. BindingExpression:Path=MaxLength; DataItem='RichTextBox' (Name=''); target element is 'TextBlock' (Name=''); target property is 'Text' (type 'String').

[jespersh]

That's true. @Keboo got an idea?

[Keboo]

I threw this in the 3.1.0 milestone as a reminder to me to give it a thorough code review when i have a little more time to concentrate on it. Will try to get to it soon.

[Keboo]

I like the look and the idea of this, however, I would like to look at options for doing this without using the error template.
I am wondering if it would be better to implement it directly on the TextBox directly, most similar to how the hint stuff is done.

[aubiyko]

I like the look and the idea of this, however, I would like to look at options for doing this without using the error template.

I don't know how to pull this off - error template wraps control, fully overlapping its lower part. So modifications are required to either enhance it with counter or compact available error text space.

I am wondering if it would be better to implement it directly on the TextBox directly, most similar to how the hint stuff is done.

You mean how it is done in SmartHint? The positive thing I see is it will remove AdornedElement binding type casting, which will result in extensibility. This may allow to merge styles while counters are added and supported by them and good default values provided via metadata override.

[Keboo]

Sorry for taking so long to get back to this. I have been holding off because I think there is a better approach for handling this. I certainly think that we want to support this, but from the material design spec the only requirement is to support the helper text.

Now with that said, the current implementation has issues. Namely it does not allow someone to implement the character count, which is certainly something people need. What I would propose is a change to the Helper text API, to enable this:

I briefly mocked something up to prove that it will work, and I am not sold on these particular APIs/names (questions below), but I think the structure is sound. The basic idea is to transition from passing purpose built, specific properties, and instead allow for arbitrary contents in the helper text portion of the template.
Something like this:

<TextBox x:Name="CommentTextBox"
            materialDesign:HintAssist.Hint="Comment"
            materialDesign:HintAssist.HelperText="Helper text">
    <materialDesign:HintAssist.HelperTemplate>
        <DataTemplate>
            <DockPanel>
                <TextBlock DockPanel.Dock="Right">
                    <TextBlock.Text>
                        <MultiBinding StringFormat="{}{0}/{1}">
                            <Binding Path="Text.Length" ElementName="CommentTextBox" />
                            <Binding Path="MaxLength" ElementName="CommentTextBox" />
                        </MultiBinding>
                    </TextBlock.Text>
                </TextBlock>
                <TextBlock Text="{Binding}" />
            </DockPanel>
        </DataTemplate>
    </materialDesign:HintAssist.HelperTemplate>
</TextBox>

Which gives this:

To do this I made three changes:

1. I changed the type of HintAssst.HelperText to be object rather than string
2. I added HintAssist.HelperTemplate attached property as a DataTemplate
3. I changed the HelperText TextBlock to be a ContentPresenter:

<ContentPresenter Canvas.Top="2" TextElement.FontSize="10" 
                  Width="{Binding ActualWidth, ElementName=border}"
                  Opacity="{Binding Path=(wpf:HintAssist.HintOpacity), RelativeSource={RelativeSource TemplatedParent}}"
                  Content="{Binding Path=(wpf:HintAssist.HelperText), RelativeSource={RelativeSource TemplatedParent}}" 
                  ContentTemplate="{Binding Path=(wpf:HintAssist.HelperTemplate), RelativeSource={RelativeSource TemplatedParent}}" />

Given that these will be breaking changes, I am going to currently flag it for the 4.0.0 milestone.

So some outstanding questions:

1. Does this approach seem reasonable? Does it solve your needs?
2. What should these attached properties be called? Using HelperText is a misleading name since it these changes enable arbitrary content. Perhaps consider HelperContent and HelperTemplate?
3. Does it make sense to keep these attached properties under HintAssist?

[aubiyko]

I certainly think that we want to support this, but from the material design spec the only requirement is to support the helper text.

That page has some inconsistency issues: character counter is only present at demo and assistive elements but is missing everywhere including specs. But I don't think that it matters. I consider character counter as right-aligned part of helper text. So it should look and feel like helper text.

1. Does this approach seem reasonable?

Most of the time suggested template should provide enough capabilities. I think it might be overkill though. Using arbitrary content there may lead to confusing user experience. That zone is called helper text for a reason.

Does it solve your needs?

Yes. @jespersh suggestions should also be incorporated in default template.

2. What should these attached properties be called? Using HelperText is a misleading name since it these changes enable arbitrary content. Perhaps consider HelperContent and HelperTemplate?

HelperContentTemplate?

3. Does it make sense to keep these attached properties under HintAssist?

Separation to HelperAssist (?) seems reasonable.

[Keboo]

Thanks for your feedback @aubiyko

I agree that allowing arbitrary content opens up the possibility of it being abused and causing poor UX. However, I do want to make sure we allow the possibility of people being able to customize the helper text without this library needing to add additional attached properties for each piece (imagine someone wanting to tweak the font size, do text wrapping, underline, etc).

Could you elaborate on what you mean by "@jespersh suggestions should also be incorporated in default template."

Thanks again

[aubiyko]

Could you elaborate on what you mean by "@jespersh suggestions should also be incorporated in default template."

Default helper content template should provide both helper text and character counter with option to show counter only when focused.

[aubiyko]

Another random thought I have: does it need to be breaking change? The best scenario would be having that new content template property defaulted with a template reading information from old HintAssist \ TextFieldAssist \ etc classes. That way:

1. People can use old properties without changing - backwards compatibility;
2. Customization is possible in ways such as complete remaking of Helper text area or altering appearance of existing information.

I've again looked through the example API you provided. Making HintAssist.HelperText as object is confusing. Anyone can make their own attached properties to have additional data.

[MichelMichels]

I'm sorry to revive maybe a dead topic, but as I was editing the spec of the TextBox and HelperText, I couldn't shake the feeling that it was weird that HelperText and HelperFontSize is a property on HintAssist. Was there a specific reason for this decision? It seems like it should be in a class of it's own, e.g. HelperAssist.

[Keboo]

@MichelMichels I would not say this is a dead topic (though it has sat idle for a while). It is a feature I would like to get into the 4.0.0 release.

As for the decision the only rational was to keep the number of *Assist classes limited. As they tend to be less discoverable to new users who may not be familiar with them. However, that goal always has to be balanced with where things make sense. There is a probably a good case to be made that the helper related stuff should probably be moved into its own class.

[Keboo]

Resolved by #2242