Skip to content

Stepper widget #308

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

Merged
merged 7 commits into from
Jan 12, 2020
Merged

Stepper widget #308

merged 7 commits into from
Jan 12, 2020

Conversation

scholtzan
Copy link
Collaborator

I did some further experimenting and implemented a stepper widget:
Screenshot 2019-11-15 at 10 18 55 AM

I tried to hook it up to a textbox but for it to work textbox would need some kind of on_change callback to update the stepper value.

@cmyr
Copy link
Member

cmyr commented Nov 17, 2019

For this to work we need some sort of 'validating textbox' that validates its inputs, and can then have it's Data be a numerical type; when an edit happens we check that the new content of the textbox is valid data; if it is we mutate the data and if it isn't we discard the edit.

@cmyr
Copy link
Member

cmyr commented Nov 17, 2019

Or rather: that's the mechanism I think this should be built on. We will need data-validation for text entry at some point anyway, so it makes sense to reuse that for this widget.

@Ralith
Copy link
Collaborator

Ralith commented Nov 26, 2019

Currently exploring such a mechanism in #346.

@cmyr
Copy link
Member

cmyr commented Dec 20, 2019

@scholtzan sorry to let this sit around for so long, it does look like a solid patch. Maybe it's worth seeing if the basic input validation stuff in #346 helps you at all?

@scholtzan
Copy link
Collaborator Author

Thanks! No worries, I've been quite busy the past couple of weeks anyway. Probably will pick it up again after Christmas.

@scholtzan
Copy link
Collaborator Author

Alright, I updated the stepper widget example and got it work with textboxes thanks to Parse.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay cool, sorry for dragging my feet on this! I have a few little observations, but nothing major; we should be able to get this merged shortly.

@scholtzan
Copy link
Collaborator Author

Ready for another look

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay a few more little nits but basically good to go!

@scholtzan
Copy link
Collaborator Author

Thanks for the comments. Should be ready for another look.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one tiny little thing but then this should be good to go. :)

@scholtzan scholtzan merged commit c40aaa1 into linebender:master Jan 12, 2020
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.

3 participants