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

Numeric up/down control added #1457

Merged
merged 12 commits into from Apr 4, 2018

Conversation

Projects
None yet
4 participants
@DmitryZhelnin
Contributor

DmitryZhelnin commented Mar 20, 2018

Ported numeric up/down control. Added sample to ControlCatalog.

default

@danwalmsley

This comment has been minimized.

Member

danwalmsley commented Mar 20, 2018

@DmitryZhelnin wow this looks really good :)

I think I missed the conversation other day about generics, related to this. but would it not make more sense have only a single control.

The values are doubles, and you have an increment property. That way the user can deal with casting the value, to byte or something else?

@DmitryZhelnin

This comment has been minimized.

Contributor

DmitryZhelnin commented Mar 20, 2018

@danwalmsley thanks for the reply! I think end-user should not be worried about casting values etc.
For example I have an int property in my viewmodel and I'll just take IntegerUpDown to manipulate this property out of the box, I dont wanna to change anything in my viewmodel.

Maybe that's a weak argument, but that's a way I used to work in WPF :)

@grokys

This comment has been minimized.

Member

grokys commented Mar 23, 2018

I think I agree with @danwalmsley, if you're using binding to bind the values then the binding system should take care of converting double <-> int etc, so you shouldn't need to change anything. Or is this not working correctly?

@DmitryZhelnin

This comment has been minimized.

Contributor

DmitryZhelnin commented Mar 24, 2018

@danwalmsley @grokys Guys, I think you were right. I'll rework the code and add new commits later.

@DmitryZhelnin DmitryZhelnin changed the title from Numeric up/down controls added to WIP: Numeric up/down controls added Mar 24, 2018

@DmitryZhelnin

This comment has been minimized.

Contributor

DmitryZhelnin commented Mar 24, 2018

Now it's only one NumericUpDown control with double values.

binding system should take care of converting double <-> int etc

This works as expected.

I updated the screenshot in the first message.

@DmitryZhelnin DmitryZhelnin changed the title from WIP: Numeric up/down controls added to Numeric up/down controls added Mar 24, 2018

@DmitryZhelnin DmitryZhelnin changed the title from Numeric up/down controls added to Numeric up/down control added Mar 24, 2018

@DmitryZhelnin DmitryZhelnin changed the title from Numeric up/down control added to WIP: Numeric up/down control added Mar 24, 2018

@DmitryZhelnin

This comment has been minimized.

Contributor

DmitryZhelnin commented Mar 24, 2018

Somehow I missed the fact that double? type of the Value property doesn't work with binded properties except they are double. So I have to use simple double for the Value and remove DefaultValue feature.

@DmitryZhelnin DmitryZhelnin changed the title from WIP: Numeric up/down control added to Numeric up/down control added Mar 24, 2018

@lindexi

This comment has been minimized.

Contributor

lindexi commented Mar 30, 2018

I think some controls should be move to other sln like UWPCommunityToolkit

@grokys

grokys approved these changes Apr 4, 2018

This looks great! As @lindexi says "advanced" controls like this should probably live somewhere else in the long run but for now lets put them into Avalonia.Controls until we decide where they should go.

@grokys

This comment has been minimized.

Member

grokys commented Apr 4, 2018

Oh actually @DmitryZhelnin I just noticed that the watermark property doesn't seem to do anything. Is that not implemented?

Ignore that, seems it does work :)

@grokys grokys merged commit 265e360 into AvaloniaUI:master Apr 4, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment