Pull Request: Adding New Control NumericUpDown Control to Material Design XAML Toolkit #3175

Asivaprasadam · 2023-04-25T20:11:54Z

Pull Request: Adding NumericUpDown control to Material Design

Preface

Hello everyone,

This is my first contribution to a GitHub project, and I am excited to be a part of the MaterialDesignXamlToolkit community. I have been working on this NumericUpDown control for Material Design, and I believe it will be a valuable addition to the project.

I must admit that my lack of experience contributing to open-source projects has delayed me in committing to this code. However, I have decided that I don't want to delay it any further by waiting to complete all the planned improvements before submitting my contribution when there is a strong community that can help me out with my 1st contribution.

Since this is my first GitHub contribution, I would appreciate any feedback and advice you can provide. I am eager to learn from more experienced developers and improve my skills, so please do not hesitate to give me constructive criticism and suggestions for improvements. I am committed to contributing to this project and look forward to continuing to learn and improve.

Thank you for taking the time to review my contribution, and I look forward to your feedback.

Description

This pull request adds a new NumericUpDown control to the Material Design project. The control provides a user-friendly way to input numeric values and is designed to fit seamlessly into the Material Design UI.

Changes Made

Created a new NumericUpDown control that allows users to input numeric values within a specific range.
Implemented the first base style for the control, nothing fancy and definitely needs improvement.
Integrated the NumericUpDown with the Material Design MainDemo and added a page to display styles.

Pending Potential Changes

Provided additional documentation to help users understand the control's functionality and usage.
Improve the support for common input methods, such as keyboard and mouse input.
Create logarithmic click/ scroll support to enhance the UX/ user experience.
Tune the base style to adhere more closely to Material Design principles.
Create additional styling templates following Material Design guidelines to provide more options for users
Add all the new styles to the Main Demo App NumericUpDown control page to showcase the control.

Screenshots

Related Issues

Additional Notes

None

Keboo

One other thing that would be good to do is write some UI tests for this control. If you need any help setting that up, let me know.

MaterialDesignThemes.Wpf/NumericUpDown.cs

Keboo · 2023-04-27T06:12:16Z

MainDemo.Wpf/NumericUpDown.xaml

+      <smtx:XamlDisplay Margin="10,5"
+                        VerticalAlignment="Top"
+                        UniqueKey="numericUpDown_default">
+        <materialDesign:NumericUpDown />


Would be nice to put in a few more examples showing the commands, Min and Max, and any other styles.

MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.NumericUpDown.xaml

Keboo · 2023-04-27T06:16:33Z

MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.NumericUpDown.xaml

+                            Padding="8,4"
+                            Command="{TemplateBinding DecreaseCommand}"/>
+
+              <TextBox x:Name="PART_TextBoxField"


We probably also want to forward the TextBox hint properties. These are attached properties that should be forwarded into the TextBox so that consumers can change them.
https://github.com/MaterialDesignInXAML/MaterialDesignInXamlToolkit/blob/master/MainDemo.Wpf/Fields.xaml#L107-L118

This is a reply to one of your comments @Keboo.

UPDATE:

While working on the TimePicker style, I think I have managed to leverage the existing TextBox styles. So for controls like these that "look" like a TextBox, we simply hijack one of the padding parameters to be able to add content on top of the TextBox but giving the appearance that the content is inside the TextBox. Of course this only really works when content is added on the far right/left but that is often the case. So for this particular control, we could leverage the TextBox style, but make the right padding big enough to fit in the plus/minus buttons. That way we get all the TextBox goodness out of the box, and simply need to add the sugar on top (the extra buttons).

My original comment:

I think, similar to some of the other controls that "host" a TextBox, we probably want to strip that particular TextBox of all the fancy MDIX stuff and implement it directly in the template for the NumericUpDown.

In other words, I assume this should eventually include all the prefix-/suffix-texts, leading-/trailing-icons, SmartHint, etc. If you look at some of the other controls (ComboBox for example), you will see that those things are handled in the "host" template, and not leveraging a "styled" TextBox.

To elaborate on the above. Today we cannot easily leverage a "styled" Textbox with the way our triggers are working today I believe, and therefore we end up having to duplicate a lot of the stuff (you'll see that in the other control styles). It may be a future refactoring task to see if we can somehow leverage a "styled" TextBox in the controls that nests one of those rather than having to handle all the fancy stuff in the "host" template.

Perfect, I will see about getting that change done.

ElieTaillard · 2023-05-06T03:09:20Z

Plus and Minus seem inverted according to your screenshot. Here are some examples of what we usually see in UI:

tim-gromeyer · 2023-11-29T10:21:28Z

Any update on this? @Asivaprasadam? Would be cool to have this in my project, this is exactly what I need!

arjun-sivaprasadam · 2024-01-10T21:19:39Z

@tim-gromeyer I have changed my work abit and switched from UI development to more of a backend development. But I'll check with the progress and do an update possibly asap, but lets say optimally by end of Jan worst case by march. Feel free to take over too!

- Auto generated control template added. - Auto generated style from Generic.xaml moved to the specific style file (MaterialDesignTheme.NumericUpDown.xaml).

Adding UI tests

nicolaihenriksen

Left a few comments to consider.

nicolaihenriksen · 2024-04-19T08:10:44Z

MaterialDesignThemes.UITests/TestBase.cs

@@ -16,7 +17,7 @@ namespace MaterialDesignThemes.UITests;

 public abstract class TestBase(ITestOutputHelper output) : IAsyncLifetime
 {
-    protected bool AttachedDebuggerToRemoteProcess { get; set; } = true;
+    protected bool AttachedDebuggerToRemoteProcess { get; set; } = false;


@Keboo please revert this before merging

nicolaihenriksen · 2024-04-19T08:21:40Z

MaterialDesignThemes.Wpf/NumericUpDown.cs

+    #region DependencyProperty : IncreaseCommandProperty
+    public ICommand? IncreaseCommand
+    {
+        get => (ICommand?)GetValue(IncreaseCommandProperty);
+        set => SetValue(IncreaseCommandProperty, value);
+    }
+    public static readonly DependencyProperty IncreaseCommandProperty =
+            DependencyProperty.Register(nameof(IncreaseCommand), typeof(ICommand), typeof(NumericUpDown), new PropertyMetadata(null));
+
+    public object? IncreaseCommandParameter
+    {
+        get => (object?)GetValue(IncreaseCommandParameterProperty);
+        set => SetValue(IncreaseCommandParameterProperty, value);
+    }
+    public static readonly DependencyProperty IncreaseCommandParameterProperty =
+        DependencyProperty.Register(nameof(IncreaseCommandParameter), typeof(object), typeof(NumericUpDown), new PropertyMetadata(null));
+
+
+    #endregion DependencyProperty : IncreaseCommandProperty
+
+    #region DependencyProperty : DecreaseCommandProperty
+    public ICommand? DecreaseCommand
+    {
+        get => (ICommand?)GetValue(DecreaseCommandProperty);
+        set => SetValue(DecreaseCommandProperty, value);
+    }
+    public static readonly DependencyProperty DecreaseCommandProperty =
+            DependencyProperty.Register(nameof(DecreaseCommand), typeof(ICommand), typeof(NumericUpDown), new PropertyMetadata(default(ICommand?)));
+
+    public object? DecreaseCommandParameter
+    {
+        get => (object?)GetValue(DecreaseCommandParameterProperty);
+        set => SetValue(DecreaseCommandParameterProperty, value);
+    }
+
+    public static readonly DependencyProperty DecreaseCommandParameterProperty =
+        DependencyProperty.Register(nameof(DecreaseCommandParameter), typeof(object), typeof(NumericUpDown), new PropertyMetadata(null));
+
+    #endregion DependencyProperty : DecreaseCommandProperty


Let me know if I am completely off the rails here, but here are my thoughts.

I think these commands are simply left-overs from when the XAML was using them. And I agree with you on-stream-comment that is not common practice for a custom control; you should rely on the template parts and deal with this stuff in the code-behind.

Also, in their current state, I really don't think they should be part of the public API for a NumericUpDown. If you look at a Slider for example, it does expose some static RoutedCommands to modify the value, but they don't have public setters. This way the calling code is not involved in determining whether Command.CanExecute returns true/false which would be a really weird thing; it is the control itself that has the knowledge to determine this, not the caller. The commands are still exposed so they can be bound to and thus executed in custom UI/flows.

nicolaihenriksen · 2024-04-19T08:27:56Z

MaterialDesignThemes.Wpf/NumericUpDown.cs

+        if (IncreaseCommand?.CanExecute(IncreaseCommandParameter) ?? false)
+            IncreaseCommand.Execute(IncreaseCommandParameter);


Related to my comment above, this feels weird to me. The command should be used to actually change the value, not be executed once the value has changed. For that, there is a ValueChanged RoutedEvent.

nicolaihenriksen · 2024-04-19T08:32:37Z

MaterialDesignThemes.Wpf/NumericUpDown.cs

+            if (e.Delta > 0)
+            {
+                OnIncrease();
+                e.Handled = true;
+            }
+            else if (e.Delta < 0)
+            {
+                OnDecrease();
+                e.Handled = true;
+            }


Not sure if it is an actual issue, but this will not mark the event as handled if e.Delta == 0 which seems a bit strange.

Imagine if the control is located in a ScrollViewer, it has keyboard focus, and the user uses the scroll wheel to change the values. Would there be a scenario where you think you are scrolling the values, but hit the edge case and suddenly the ScrollViewer scrolls instead because the event bubbles up? That would be annoying!

nicolaihenriksen · 2024-04-19T08:40:52Z

MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.NumericUpDown.xaml

+                            Padding="8,4"
+                            Command="{TemplateBinding DecreaseCommand}"/>
+
+              <TextBox x:Name="PART_TextBoxField"


This is a reply to one of your comments @Keboo.

UPDATE:

While working on the TimePicker style, I think I have managed to leverage the existing TextBox styles. So for controls like these that "look" like a TextBox, we simply hijack one of the padding parameters to be able to add content on top of the TextBox but giving the appearance that the content is inside the TextBox. Of course this only really works when content is added on the far right/left but that is often the case. So for this particular control, we could leverage the TextBox style, but make the right padding big enough to fit in the plus/minus buttons. That way we get all the TextBox goodness out of the box, and simply need to add the sugar on top (the extra buttons).

My original comment:

I think, similar to some of the other controls that "host" a TextBox, we probably want to strip that particular TextBox of all the fancy MDIX stuff and implement it directly in the template for the NumericUpDown.

In other words, I assume this should eventually include all the prefix-/suffix-texts, leading-/trailing-icons, SmartHint, etc. If you look at some of the other controls (ComboBox for example), you will see that those things are handled in the "host" template, and not leveraging a "styled" TextBox.

To elaborate on the above. Today we cannot easily leverage a "styled" Textbox with the way our triggers are working today I believe, and therefore we end up having to duplicate a lot of the stuff (you'll see that in the other control styles). It may be a future refactoring task to see if we can somehow leverage a "styled" TextBox in the controls that nests one of those rather than having to handle all the fancy stuff in the "host" template.

nicolaihenriksen · 2024-04-19T08:43:14Z

MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.NumericUpDown.xaml

+                       HorizontalContentAlignment="{TemplateBinding HorizontalContentAlignment}" />
+
+              <RepeatButton x:Name="PART_DecreaseButton"
+                            Content="{local:PackIcon Kind=Minus}"


I think the PackIcons used for the plus/minus buttons should be exposed as dependency properties defaulting to the values you have set here. That gives a caller better control of what to show in the button.

Possibly it shouldn't even be a PackIcon but simply an object in case we want callers to be able to add arbitrary content to these buttons.

That is a good point, just expose the button's content and let consumers do what they like with it.

nicolaihenriksen · 2024-04-19T08:50:09Z

MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.NumericUpDown.xaml

+
+              <RepeatButton x:Name="PART_DecreaseButton"
+                            Content="{local:PackIcon Kind=Minus}"
+                            Style="{StaticResource MaterialDesignFlatButton}"


If/when we introduce more styles for this control (e.g. "Floating", "Filled", and "Outlined"), the MaterialDesignFlatButton style hardcoded here should probably be a parameter passed into the base style (internal DP/AP?).

Alternatively a dedicated RepeatButtonStyle can be passed in from the style-override.

nicolaihenriksen · 2024-04-19T08:52:16Z

MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.NumericUpDown.xaml

+    </Setter>
+  </Style>
+
+  <Style TargetType="wpf:NumericUpDown" BasedOn="{StaticResource MaterialDesignNumericUpDown}" />


These default implicit styles for the custom controls are typically set up in Generic.xaml; see here.

However, I actually prefer them to be here directly in the style. This is where the responsibility lies, and these resource dictionaries are pulled into Generic.xaml anyway. To strengthen this argument, as part of adopting the SmartHint in TimePicker, I am nesting a TextBox and thus need to forward a style. That just pollutes Generic.xaml with stuff like this which really belongs closer to the other explicit styles.

Asivaprasadam marked this pull request as ready for review April 25, 2023 20:24

Asivaprasadam marked this pull request as draft April 25, 2023 20:28

Keboo self-requested a review April 25, 2023 21:36

Keboo reviewed Apr 27, 2023

View reviewed changes

Keboo added this to the 4.9.0 milestone Apr 27, 2023

Keboo modified the milestones: 4.9.0, 5.0.0 May 11, 2023

Keboo modified the milestones: 5.0.0, 5.1.0 Jan 25, 2024

Arjun Sivaprasadam and others added 5 commits April 17, 2024 21:52

Adding bare bones for the NumericUpDown control!

687cb8b

- Auto generated control template added. - Auto generated style from Generic.xaml moved to the specific style file (MaterialDesignTheme.NumericUpDown.xaml).

Almost finalized custom control and initial styling added!

dccd824

Changes to display initial design on the MainDemo!

4382940

Some working changes from tonight's live stream

1a4038f

Cleaning up the numeric up/down control

dfe8f13

Adding UI tests

Keboo force-pushed the feature/NumericUpDown branch from 799b4e4 to dfe8f13 Compare April 18, 2024 06:06

Keboo marked this pull request as ready for review April 18, 2024 06:06

Keboo requested a review from nicolaihenriksen April 18, 2024 06:06

Forwarding alignment properties

defe04b

nicolaihenriksen reviewed Apr 19, 2024

View reviewed changes

Keboo added enhancement release notes Items are likely to be highlighted in the release notes. labels Apr 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pull Request: Adding New Control NumericUpDown Control to Material Design XAML Toolkit #3175

Pull Request: Adding New Control NumericUpDown Control to Material Design XAML Toolkit #3175

Asivaprasadam commented Apr 25, 2023 •

edited

Keboo left a comment

Keboo Apr 27, 2023

Keboo Apr 27, 2023

nicolaihenriksen Apr 19, 2024 •

edited

Keboo Apr 21, 2024

ElieTaillard commented May 6, 2023

tim-gromeyer commented Nov 29, 2023

arjun-sivaprasadam commented Jan 10, 2024 •

edited

nicolaihenriksen left a comment

nicolaihenriksen Apr 19, 2024

nicolaihenriksen Apr 19, 2024 •

edited

nicolaihenriksen Apr 19, 2024

nicolaihenriksen Apr 19, 2024 •

edited

nicolaihenriksen Apr 19, 2024 •

edited

nicolaihenriksen Apr 19, 2024

Keboo Apr 21, 2024

nicolaihenriksen Apr 19, 2024

nicolaihenriksen Apr 19, 2024 •

edited

		if (IncreaseCommand?.CanExecute(IncreaseCommandParameter) ?? false)
		IncreaseCommand.Execute(IncreaseCommandParameter);

Pull Request: Adding New Control NumericUpDown Control to Material Design XAML Toolkit #3175

Are you sure you want to change the base?

Pull Request: Adding New Control NumericUpDown Control to Material Design XAML Toolkit #3175

Conversation

Asivaprasadam commented Apr 25, 2023 • edited

Pull Request: Adding NumericUpDown control to Material Design

Preface

Description

Changes Made

Pending Potential Changes

Screenshots

Related Issues

Additional Notes

Keboo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolaihenriksen Apr 19, 2024 • edited

Choose a reason for hiding this comment

UPDATE:

Choose a reason for hiding this comment

ElieTaillard commented May 6, 2023

tim-gromeyer commented Nov 29, 2023

arjun-sivaprasadam commented Jan 10, 2024 • edited

nicolaihenriksen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolaihenriksen Apr 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolaihenriksen Apr 19, 2024 • edited

Choose a reason for hiding this comment

nicolaihenriksen Apr 19, 2024 • edited

Choose a reason for hiding this comment

UPDATE:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolaihenriksen Apr 19, 2024 • edited

Choose a reason for hiding this comment

Asivaprasadam commented Apr 25, 2023 •

edited

nicolaihenriksen Apr 19, 2024 •

edited

arjun-sivaprasadam commented Jan 10, 2024 •

edited

nicolaihenriksen Apr 19, 2024 •

edited

nicolaihenriksen Apr 19, 2024 •

edited

nicolaihenriksen Apr 19, 2024 •

edited

nicolaihenriksen Apr 19, 2024 •

edited