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

dragging between different workspaces #1579

Conversation

macherel
Copy link

@macherel macherel commented Jan 30, 2018

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Allow to move blocks from one workspace to another using drag/drop

Proposed Changes

When dragging a block, if the mouse is over another workspace

  • copy the block to the new workspace,
  • stop dragging old block (dispatch mouseup event)
  • dispose the old block
  • start dragging the new block (dispatch mousedown and mousemove events)

Reason for Changes

Test Coverage

Tested on:

  • Desktop Chrome
  • Desktop Firefox
  • Windows Internet Explorer 11

Additional Information

@AnmAtAnm
Copy link
Contributor

I will leave it to Rachel to determine if this is how we want to approach this problem.

Please review the style guide and correct the code. The included ESLint rules should help you identify problems.

@macherel
Copy link
Author

About ESLint rules (more particularly indent.callexpression), it appears to be inconsistent with the rules that are verified by Travis.

@rachel-fenichel
Copy link
Collaborator

Travis runs with the rules in our .eslintrc.

@macherel
Copy link
Author

macherel commented Feb 1, 2018

There's no rule for indent.callexpression in .eslintrc and default value is "1"
Before my last commit, my code respect this rule but Travis job fails. I had to change indentation to pass CI (as it was explained in the Travis errors).
Apart from that, I didn't see any other styling errors

@rachel-fenichel
Copy link
Collaborator

The rule for callexpression: https://github.com/google/blockly/blob/develop/.eslintrc#L20

"Apart from that, I didn't see any other styling errors."

Some of these are things that you can see by looking at the style of the surrounding code, without even reading the style guide.

@macherel
Copy link
Author

macherel commented Feb 2, 2018

Thank you for your feedback
My branch was not up to date from upstream branch.
Concerning horizontal whitespace, I propose to add "keyword-spacing": ["error"] to eslintrc
And line wrap isset to 120 in eslintrc

@macherel macherel force-pushed the dragging_blocks_between_workspaces_using_mouseevent branch from 053d033 to 3dd2539 Compare February 2, 2018 10:01
@rachel-fenichel rachel-fenichel mentioned this pull request Mar 19, 2018
3 tasks
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