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

Remove 'Create a Behavior/Function' buttons from the toolbox WYSIWYG levelbuilder page #32533

Merged
merged 4 commits into from
Jan 13, 2020

Conversation

jmkulwik
Copy link
Contributor

@jmkulwik jmkulwik commented Jan 6, 2020

Description

Currently, the 'Create a Function/Behavior' buttons in /edit_blocks/toolbox_blocks do not work. This is intentional - levelbuilders should not be able to add a custom behavior or function in /edit_blocks/toolbox_blocks mode. This PR removes the buttons from /edit_blocks/toolbox_blocks page.

Links

This is the button removed as part of this change:
image

Testing story

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@jmkulwik jmkulwik requested a review from a team as a code owner January 6, 2020 19:26
@jmkulwik jmkulwik changed the title Remove custom behavior button Remove 'Create a Behavior/Function' buttons from the toolbox WYSIWYG levelbuilder page Jan 6, 2020
@@ -2682,7 +2682,8 @@ StudioApp.prototype.handleUsingBlockly_ = function(config) {
false
),
useModalFunctionEditor: utils.valueOr(
config.level.useModalFunctionEditor,
config.level.edit_blocks !== TOOLBOX_EDIT_MODE &&
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems overcomplicated? Is config.level.useModalFunctionEditor a boolean? If so, then isn't this equivalent to just useModalFunctionEditor: config.level.edit_blocks !== TOOLBOX_EDIT_MODE && config.level.useModalFunctionEditor (no need for utils.valueOr)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the reason for using utils.valueOr is that config.level.useModalFunctionEditor can be undefined whereas we always want useModalFunctionEditor to be T/F.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I think you want like, edit_blocks !== TOOLBOX && !!useModalFunctionEditor
To me it seems like a little bit of a code smell to use valueOr with a boolean as the default value, because the boolean expression can almost certainly be simplified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 (thanks for the offline conversation) Also added a jira ticket with more details about future plans! https://codedotorg.atlassian.net/browse/STAR-944

@@ -2707,7 +2708,8 @@ StudioApp.prototype.handleUsingBlockly_ = function(config) {
// Never show unused blocks or disable autopopulate in edit mode
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: can you update this comment now that we are sometimes disabling autopopulate?

@jmkulwik jmkulwik force-pushed the remove-custom-behavior-button branch from be69cf2 to 0d36d1b Compare January 13, 2020 18:51
@jmkulwik jmkulwik merged commit 9f43dba into staging Jan 13, 2020
@jmkulwik jmkulwik deleted the remove-custom-behavior-button branch January 13, 2020 22:55
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.

None yet

2 participants