Skip to content
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

Add NumberInput widget #11

Merged
merged 6 commits into from
May 4, 2021
Merged

Add NumberInput widget #11

merged 6 commits into from
May 4, 2021

Conversation

hangleang
Copy link
Contributor

Hi there. This is the implementation of NumberInput widget. Sorry for committing lately, I was very busy on my work recently.

Feel free to give me some idea to improve this widget. Thanks you guys....

@Kaiden42
Copy link
Collaborator

Thanks for your PR! 🙂

Sorry for committing lately, I was very busy on my work recently.

Don't worry, I was busy as well.

Looks pretty good so far. However, I have two small suggestions for improvement:
I saw that the new number_input widget depends on the crate num-traits. Could you, similar to the crate chrono, mark this as optional? For the number_input feature you can then say that it depends on the num-traits "feature". Similar to the date_picker feature. This way the dependency will only be compiled if the number_input feature is enabled. You can read more about this here.

Additionally, have you thought about floating point support? These are currently only partially supported. I was able to change the type of the number_input example from u8 to f32, but it caused strange effects. I am not able to enter a decimal point into the field myself, but I am able to generate floating point numbers via the increment/decrement by setting the step size to 0.5. If I then delete the decimal point, I even get beyond the maximum value.
Example: I set the value to 26 and increment by 0.5. This gives me 26.5. If I now remove the decimal point I am at 265, whereas only a maximum of 255.0 is allowed.

@hangleang
Copy link
Contributor Author

Done 😀

@Kaiden42 Kaiden42 merged commit bad0ace into iced-rs:main May 4, 2021
@Kaiden42
Copy link
Collaborator

Kaiden42 commented May 4, 2021

Awesome! Thank you for your work. 🙂

I've now merged your PR. However, I noticed two minor issues, but they can be addressed in the future.

  • With decimal support I'm not able to type the decimalpoint at the end of the number. This could be caused by the cut-off of the ".0" decimal.
  • Support for web would be great in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants