Skip to content

Add glide (number) secs to [drop down] block #1046

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 4 commits into from
Aug 24, 2017
Merged

Add glide (number) secs to [drop down] block #1046

merged 4 commits into from
Aug 24, 2017

Conversation

Kenny2github
Copy link
Contributor

@Kenny2github Kenny2github commented Aug 10, 2017

Resolves

scratchfoundation/scratch-gui#598

Proposed Changes

Adds the “glide (number) secs to [drop down]” block to motion.js and default_toolbox.js

Comes with scratchfoundation/scratch-vm#662 and scratchfoundation/scratch-gui#634

@Kenny2github
Copy link
Contributor Author

Kenny2github commented Aug 15, 2017

@ericrosenbaum seeing as you’re requested for review, any comments? /cc @thisandagain

@Kenny2github
Copy link
Contributor Author

@ericrosenbaum updates? /cc @towerofnix

@towerofnix
Copy link
Contributor

towerofnix commented Aug 21, 2017

These changes look good to me (though I'm obviously not one of the assigned reviewers!). (Also, it's awesome that the order of definitions in motion.js is still consistent, i.e. "menus" then "blocks".)

Copy link
Contributor

@ericrosenbaum ericrosenbaum 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 to me!

@Kenny2github
Copy link
Contributor Author

Kenny2github commented Aug 22, 2017

@towerofnix Yes, I noticed that and made the change comply with that convention :)

Alright then - merge when you will :D

@ericrosenbaum
Copy link
Contributor

One more fix- the default value for seconds, 1, is not getting read in correctly. We just need to change field name="SECS" to field name="NUM"

@Kenny2github
Copy link
Contributor Author

@ericrosenbaum Done! Merge when you will!

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

Successfully merging this pull request may close these issues.

5 participants