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

Feature/merge feb 2017 #791

Merged
merged 272 commits into from Feb 21, 2017

Conversation

Projects
None yet
@rachel-fenichel
Collaborator

rachel-fenichel commented Feb 15, 2017

Merge google/blockly develop into scratch-blocks develop.

rachel-fenichel and others added some commits Nov 9, 2016

Merge pull request #716 from nilq/master
Rename README to README.md
Merge pull request #729 from rachel-fenichel/feature/import_definitions
Add ability to define blocks with a json array
Merge pull request #744 from AnmAtAnm/temp
Unblock push to master.
Merge pull request #745 from rachel-fenichel/merge_master_to_develop_…
…nov_11_16

Merge master to develop nov 11 16
Merge pull request #736 from google/fraser-develop
Allow images in dropdown menus.
Improve performance of block dragging. This is a backport of the blo… (
…#732)

Improve performance of block dragging.  This is a backport of the block drag surface from scratch-blocks.  At the beginning of a block drag, blocks get moved to a drag surface which then translates using translate3d to avoid repainting the entire svg on every mouse move.  At the end of the drag, the blocks are dropped back in the svg in their new position.
API-breaking cleanup. But doubtful anyone will be affected. (#748)
* Make add/removeClass return whether they did anything.
* Move more functions onto utils.
* Move bind functions to Blockly.
* Routine recompile.
String reference in JSON string messages (#741)
 * Adds message references to message string interpolation, in the form of %{BKY_STRING}.
 * Re-adding CONTROLS_IFELSE block using the new syntax, referencing to CONTROL_IF equivalents.
Move away from using a common modal service, since the block options …
…and the toolbox modals are going to end up behaving fairly differently.
@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 15, 2017

Collaborator

6 jsunit tests fail--I'm looking into them.

@thisandagain @rschamp please sanity check basic functionality.

Collaborator

rachel-fenichel commented Feb 15, 2017

6 jsunit tests fail--I'm looking into them.

@thisandagain @rschamp please sanity check basic functionality.

@rachel-fenichel rachel-fenichel requested a review from picklesrus Feb 15, 2017

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 15, 2017

Collaborator

Data category doesn't work (probably missing a call to register a toolbox category)

Collaborator

rachel-fenichel commented Feb 15, 2017

Data category doesn't work (probably missing a call to register a toolbox category)

@thisandagain thisandagain added this to the March 23 milestone Feb 15, 2017

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 15, 2017

Collaborator

Data category population fixed.

RTL text rendering is a bit weird:
image

Collaborator

rachel-fenichel commented Feb 15, 2017

Data category population fixed.

RTL text rendering is a bit weird:
image

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 15, 2017

Collaborator

The problem with renaming and deleting variables is still there, but I want to fix that separately.

Collaborator

rachel-fenichel commented Feb 15, 2017

The problem with renaming and deleting variables is still there, but I want to fix that separately.

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 15, 2017

Collaborator

Unit tests fixed.

@rschamp there are some changes to package.json that may be wrong and/or blocking Travis, but I don't know enough to debug them.

Other than the RTL text input rendering, I think this is all good.

Collaborator

rachel-fenichel commented Feb 15, 2017

Unit tests fixed.

@rschamp there are some changes to package.json that may be wrong and/or blocking Travis, but I don't know enough to debug them.

Other than the RTL text input rendering, I think this is all good.

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 15, 2017

Collaborator

RTL text inputs fixed.

Collaborator

rachel-fenichel commented Feb 15, 2017

RTL text inputs fixed.

@thisandagain

This comment has been minimized.

Show comment
Hide comment
@thisandagain

thisandagain Feb 16, 2017

Member

It looks like travis is unhappy due to eslint issues with the tests:
https://travis-ci.org/LLK/scratch-blocks/jobs/202054431

I think we could just make exceptions here to the no-undef and no-unused-vars rules to get things passing again.

Member

thisandagain commented Feb 16, 2017

It looks like travis is unhappy due to eslint issues with the tests:
https://travis-ci.org/LLK/scratch-blocks/jobs/202054431

I think we could just make exceptions here to the no-undef and no-unused-vars rules to get things passing again.

@thisandagain

This comment has been minimized.

Show comment
Hide comment
@thisandagain

thisandagain Feb 16, 2017

Member

Extremely minor, but I noticed that we now have a stroke around the insertion shadow.

Member

thisandagain commented Feb 16, 2017

Extremely minor, but I noticed that we now have a stroke around the insertion shadow.

@thisandagain

This comment has been minimized.

Show comment
Hide comment
@thisandagain

thisandagain Feb 16, 2017

Member

In the horizontal blocks:
If you edit a numeric field, the field will return to its original value on blur.

Member

thisandagain commented Feb 16, 2017

In the horizontal blocks:
If you edit a numeric field, the field will return to its original value on blur.

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 16, 2017

Collaborator

"In the horizontal blocks:
If you edit a numeric field, the field will return to its original value on blur."
@picklesrus and I have both noticed this as well, but she has it on a version of scratch-blocks pre-merge. I don't think it's specific to this merge.

"Extremely minor, but I noticed that we now have a stroke around the insertion shadow."
Meaning there should be one and isn't, or shouldn't be one and is?

RE: no-undef: were those rules ever being applied to the unit tests, or is it just that the new unit tests aren't set up to be exceptions and the old ones were?

Collaborator

rachel-fenichel commented Feb 16, 2017

"In the horizontal blocks:
If you edit a numeric field, the field will return to its original value on blur."
@picklesrus and I have both noticed this as well, but she has it on a version of scratch-blocks pre-merge. I don't think it's specific to this merge.

"Extremely minor, but I noticed that we now have a stroke around the insertion shadow."
Meaning there should be one and isn't, or shouldn't be one and is?

RE: no-undef: were those rules ever being applied to the unit tests, or is it just that the new unit tests aren't set up to be exceptions and the old ones were?

@thisandagain

This comment has been minimized.

Show comment
Hide comment
@thisandagain

thisandagain Feb 16, 2017

Member

Horizontal Block Inputs

Ok! Happy to file an issue and follow-up after.

Insertion Shadow

There is no stroke on insertion shadows in develop (and design spec) and this PR introduces a stroke.

Lint Ignore

Looks like you found it. :-)

Member

thisandagain commented Feb 16, 2017

Horizontal Block Inputs

Ok! Happy to file an issue and follow-up after.

Insertion Shadow

There is no stroke on insertion shadows in develop (and design spec) and this PR introduces a stroke.

Lint Ignore

Looks like you found it. :-)

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 16, 2017

Collaborator

Insertion shadow: Fixed, I think. Looks like that wasn't quite right before, but two bugs combined to become a feature, and I fixed one of those bugs in this PR.

I think I'm all set. If you're happy with this, please squash and merge it in. Then I can write some extension demos for you.

Collaborator

rachel-fenichel commented Feb 16, 2017

Insertion shadow: Fixed, I think. Looks like that wasn't quite right before, but two bugs combined to become a feature, and I fixed one of those bugs in this PR.

I think I'm all set. If you're happy with this, please squash and merge it in. Then I can write some extension demos for you.

@thisandagain

When testing in the VM playground, we get a fatal exception on the line:

var flyoutWorkspace = workspace.getFlyout().getWorkspace();
Uncaught TypeError: workspace.getFlyout is not a function
    at window.onload (playground.js:64)

Did the API for this change? @rschamp can you please review this as well?

@rschamp

This comment has been minimized.

Show comment
Hide comment
@rschamp

rschamp Feb 20, 2017

Member

I see the same thing. It looks like getFlyout was made private in workspace, and should be implemented by subclasses. getFlyout was removed from workspace_svg. I think we could just un-remove it from workspace_svg?

Member

rschamp commented Feb 20, 2017

I see the same thing. It looks like getFlyout was made private in workspace, and should be implemented by subclasses. getFlyout was removed from workspace_svg. I think we could just un-remove it from workspace_svg?

@rachel-fenichel

This comment has been minimized.

Show comment
Hide comment
@rachel-fenichel

rachel-fenichel Feb 21, 2017

Collaborator

Not quite--we failed at the merge last time and ended up with two versions of getFlyout_ in workspace_svg:
https://github.com/LLK/scratch-blocks/blob/develop/core/workspace_svg.js#L648
https://github.com/LLK/scratch-blocks/blob/develop/core/workspace_svg.js#L528

I removed one to dedupe it.

I can fix this for y'all by making workspace:getFlyout and workspace_svg:getFlyout public, though that's out of sync with mainline blockly. Is that the fix you want?

Collaborator

rachel-fenichel commented Feb 21, 2017

Not quite--we failed at the merge last time and ended up with two versions of getFlyout_ in workspace_svg:
https://github.com/LLK/scratch-blocks/blob/develop/core/workspace_svg.js#L648
https://github.com/LLK/scratch-blocks/blob/develop/core/workspace_svg.js#L528

I removed one to dedupe it.

I can fix this for y'all by making workspace:getFlyout and workspace_svg:getFlyout public, though that's out of sync with mainline blockly. Is that the fix you want?

@thisandagain

This comment has been minimized.

Show comment
Hide comment
@thisandagain
Member

thisandagain commented Feb 21, 2017

@thisandagain thisandagain merged commit 57c3666 into LLK:develop Feb 21, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rachel-fenichel rachel-fenichel deleted the rachel-fenichel:feature/merge_feb_2017 branch Feb 21, 2017

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