Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Added new drag hover and drop states. #391

Merged
merged 1 commit into from Jun 20, 2014
Merged

Conversation

huwshimi
Copy link
Member

Added new drop states and highlighting when tokens are dragged over drop targets.

QA:

  • drag two charms to the canvas
  • open the machine view
  • start dragging one of the units
  • the 'Create new machine' should appear in the header with new styling
  • hover the unit over the drop area
  • the background should go grey
  • drop the unit on the header and click create in the constraints form
  • click on the 'new0' machine
  • start dragging the second unit
  • the 'Create new machine' and 'Create new container' headers' targets should appear
  • the machine and container tokens should show the 'Add to machine new0' and 'Add to container Bare metal' respectively
  • hovering the unit over the headers and tokens should turn them grey
  • dropping the unit on a header or token should make the drop areas disappear

@jujugui
Copy link
Contributor

jujugui commented Jun 18, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1249/

container.delegate('drop', this._unitDropHandler, spec, this);
container.delegate('dragenter', this._ignore, spec, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

By removing this you will break drop events because the dragenter event needs to be prevented to allow the drop events to fire. See https://developer.mozilla.org/en-US/docs/DragDrop/Drag_Operations#droptargets

@jujugui
Copy link
Contributor

jujugui commented Jun 19, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1253/

@jujugui
Copy link
Contributor

jujugui commented Jun 19, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1254/

@jujugui
Copy link
Contributor

jujugui commented Jun 19, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1255/

@@ -45,9 +45,10 @@ YUI.add('mv-drop-target-view-extension', function(Y) {
_attachDragEvents: function(spec) {
var container = this.get('container');
// .token is the container of the machine/container token
spec = spec || '.token';
spec = spec || '.drop';
Copy link
Contributor

Choose a reason for hiding this comment

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

The machine-panel-view-header.js called this method passing in the spec as '.drop'. Since it's now the default just for a little cleanup that method no longer needs to pass it in.

@hatched
Copy link
Contributor

hatched commented Jun 19, 2014

Thanks for this, it's coming together nicely!
👍 With the small changes mentioned above
QA OK

@jujugui
Copy link
Contributor

jujugui commented Jun 19, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1257/

@jujugui
Copy link
Contributor

jujugui commented Jun 19, 2014

Test FAILed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1259/

@huwshimi
Copy link
Member Author

The last commit fixes dropping a unit on a token, but as part of that I needed to move the event attaching out of the render as they were being attached multiple times. Before I fix all the tests I wanted to check why it was being done that way in the first place and if this is an OK solution?

@jujugui
Copy link
Contributor

jujugui commented Jun 20, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1273/

@huwshimi
Copy link
Member Author

Enough has changed in this branch that it could probably do with another review.

@kadams54
Copy link
Contributor

QA:

  • If a machine or container is added (i.e., someone else making changes) in mid-drag, the new token is missing the "drop zone" styling. Easy way to duplicate in local dev is to deploy a slew of services and then drag something and hold it until the simulator starts creating new machines. I suspect we may want to address this when we fix other similar bugs, like newly-created container tokens showing up under the wrong machine.
  • What are the performance implications of changing the styling on all of these tokens for environments that have lots and lots of machines or containers? I let the simulation run for awhile and tried with 30+ machines and didn't notice any issues.

this._machinesHeader.setDroppable();
// We only show that the container header is droppable if the user
// has selected a machine as a parent already.
if (this.get('selectedMachine')) {
this._containersHeader.setDroppable();
}
// Show the drop states for all visible machines and containers.
Object.keys(machineTokens).forEach(function(id) {
var token = machineTokens[id];
Copy link
Contributor

Choose a reason for hiding this comment

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

redefining a var every loop

machineTokens[id].setDroppable();

(The same is done 4 times)

@hatched
Copy link
Contributor

hatched commented Jun 20, 2014

👍 Again :)
Thanks for keeping at this one.
:shipit:

@jujugui
Copy link
Contributor

jujugui commented Jun 20, 2014

Build failed: Attempt to land pull request failed
build url: http://ci.jujugui.org:8080/job/juju-gui-merge/415

@jujugui
Copy link
Contributor

jujugui commented Jun 20, 2014

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge

jujugui added a commit that referenced this pull request Jun 20, 2014
Added new drag hover and drop states.

Added new drop states and highlighting when tokens are dragged over drop targets.

QA:
- drag two charms to the canvas
- open the machine view
- start dragging one of the units
- the 'Create new machine' should appear in the header with new styling
- hover the unit over the drop area
- the background should go grey
- drop the unit on the header and click create in the constraints form
- click on the 'new0' machine
- start dragging the second unit
- the 'Create new machine' and 'Create new container' headers' targets should appear
- the machine and container tokens should show the 'Add to machine new0' and 'Add to container Bare metal' respectively
- hovering the unit over the headers and tokens should turn them grey
- dropping the unit on a header or token should make the drop areas disappear
@jujugui jujugui merged commit 8406ff7 into juju:develop Jun 20, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants