Added clone and remove entity to the Edit menu #197

fernandojsg · 2016-07-12T15:45:45Z

Added clone and remove entity to the Edit menu
ID Collision detection that will add an incremental suffix
Remove entity ask for confirmation
Menu => ExportMenu, CreateMenu and EditMenu
Fixes Clone entity #36 and Purpose of "Edit" menu? #13

ngokevin · 2016-07-13T17:40:26Z

src/lib/editor.js

      this.addObject(event.target.object3D);
    }.bind(this));

+    document.addEventListener('componentchanged', function(event) {


event => { ... } will give you the binding

ngokevin · 2016-07-13T18:29:45Z

Is the Edit Menu the best place? There's a lot of distance between the Entity in the SceneGraph to the Edit Menu, so the mapping is not immediate. Can we move these actions to the SceneGraph (right-click contextual menu or action icons)

fernandojsg · 2016-07-13T18:39:21Z

@ngokevin regardless the position on the UI I think it's good idea to keep all the options in the menu too, so people could access them even if some of the panels are collapsed for example.
In any case I also think the best place to have the icons is also in the scenegraph, maybe you could click an entity and the delete button will be aligned to the right in the same row.
I also like the contentmenu option, probably even on the viewport too, so you could click on the entities and see some options related to them.
This PR is more about adding the functionality, and then I could add another two one for context menu and another for icons in scenegraph.

ngokevin · 2016-07-13T18:51:05Z

We don't have collapsible panels at the moment, so right now it's not ideal to address that case. I think even then, we'd just prescribe to open up the Scene Graph to do entity-level management.

dmarcos · 2016-07-13T18:56:43Z

What about a contextual menu (with clone, delete) as well on the viewport when right-click on an entity?

dmarcos · 2016-07-13T19:01:13Z

If we add the delete, clone on both the viewport and the scene graph we might be able to get rid of the edit menu. Edit menu feels indirect while a contextual menu / icon feels more like direct manipulation which reinforces the mapping between action and target of that action.

fernandojsg · 2016-07-13T20:50:03Z

Yep, I was more on the way to keep all the actions on the menus as most of the apps does, like firefox you've context menu to copy, paste, whatever but you have also shortcuts and at the end you can always go to the menu.
I believe for me the most confortable will be probably contextmenu on viewport and shortcuts but this is more for "power" users.
About adding icons to the scenegraph, probably delete it's the most common that you can have there, and maybe clone too?
What I mean is that there's no problem in duplicate places where you can do the same action so isn't? so we could have them all in the menu

fernandojsg · 2016-07-13T23:25:15Z

Btw from the discussion in #203
Do you think we should include the clone icon too in next to the delete icon on the selected entity?

dmarcos · 2016-07-13T23:30:50Z

How do you create a child of a given entity? If you select one and click on the + icon?

dmarcos · 2016-07-13T23:32:48Z

What about copy and paste for entities instead of clone? You copy an entity and can click on another one to paste it as a child. That would allow for easy creation of hierarchies.

fernandojsg · 2016-07-13T23:41:26Z

Copy/paste should be on the contextmenu instead of icons right?
I'm not sure about to create a child of the selected entity with just pressing +.
Because typically you'll have always one entity selected (instead of a-scene) Unity/3DS and so on typically works that way, you just add an entity at the end of the scenegraph and then you decide where to locate it.
One option could be having two + buttons one to add an entity at the end of the scenegraph and another one (just enabled if one entity is selected) to add an entity inside the current selected entity.
Another option is that + will be always add at the end, and we could add an option in the contextmenu for each entity to add a child entity (or whatever)

dmarcos · 2016-07-13T23:48:39Z

Copy / paste could be accessed on:

Keyboard shortcut ctrl+c / ctrl + v.
Contextual menu or icons associated to the entity.
Maybe also next to the + icon in the scenegraph.

We can also add a create child and paste actions to the contextual menu of the entity.

fernandojsg · 2016-07-13T23:56:41Z

Ok, so I'll split all this things into different issues and PR and we could keep discussing them there, as there many little things going here

fernandojsg · 2016-07-13T23:57:59Z

What do we do with this PR? Just merge the way it works right now with the Edit menu? and create another PR adding the clone and remove buttons on the �scenegraph`s entity, or modify this PR to include these button too?

dmarcos · 2016-07-14T00:01:51Z

Are you going to include html elment + id in the scenegraph in this PR?

dmarcos · 2016-07-14T00:02:27Z

We can add clone and remove in this PR too I think.

fernandojsg · 2016-07-14T00:03:25Z

Are you going to include html elment + id in the scenegraph in this PR?

No, I'll leave that for another PR

fernandojsg · 2016-07-14T00:05:08Z

We can add clone and remove in this PR too I think.

Ok, so I'll add clone and remove buttons next in the scenegraph in this PR and leave the rest for other PRs/issues

fernandojsg · 2016-07-14T15:50:23Z

Btw how do you feel about select the newly created element after clone, or keeping selected the element to be clone?

dmarcos · 2016-07-14T16:46:35Z

I think + or clone should select the new element.

fernandojsg · 2016-07-14T23:33:54Z

Updated with the clone and remove icon added to the scenegraph. When cloning an entity it will select the new entity by default and when removing it will select the closest entity (first looking forward and then backward):

ngokevin · 2016-07-15T00:20:12Z

Do we keep the Edit Menu? Don't know if we want more than one way to do something.

ngokevin · 2016-07-15T00:20:35Z

src/actions/entity.js

+  }
+}
+
+function findClosestEntity(entity) {


ngokevin · 2016-07-15T00:24:36Z

r+wc

fernandojsg · 2016-07-15T09:37:38Z

@ngokevin re leaving the options in the edit menu, let's leave them there until we'll decide which icons we'll show on the scenegraph. As one of the next things that I'll do is to copy/paste, probably we don't want to have copy, paste, clone and delete in the scenegraph, so we could arrange it in another PR/issue

ngokevin · 2016-07-15T17:14:56Z

The same thing still applies to copy/paste though, you want to keep the direct mapping between the entity and its actions.

fernandojsg · 2016-07-15T17:21:05Z

Yep, but what if we have like 10 actions for the entity, we should leave some of them in the context menu and just the two/three more common on the icons next to the entity data on the scenegraph right?

ngokevin · 2016-07-15T17:28:30Z

Maybe, maybe not. But that's assuming we have 10 actions, which we don't at the moment.

Added clone and remove entity to the Edit menu

fernandojsg added the needs review label Jul 12, 2016

ngokevin reviewed Jul 13, 2016
View reviewed changes

fernandojsg force-pushed the delete branch from 3e1bcec to 81407b6 Compare July 13, 2016 18:35

fernandojsg added work-in-progress and removed needs review labels Jul 13, 2016

fernandojsg force-pushed the delete branch from 56c0b37 to 6024691 Compare July 14, 2016 23:22

Added clone and remove entity to the Edit menu

d8dc74d

fernandojsg force-pushed the delete branch from 6024691 to 80da443 Compare July 14, 2016 23:31

fernandojsg added needs review and removed work-in-progress labels Jul 14, 2016

ngokevin reviewed Jul 15, 2016
View reviewed changes

fernandojsg force-pushed the delete branch from 80da443 to 5d6998b Compare July 15, 2016 09:34

Added clone and remove icons on scenegraph

a56578f

fernandojsg force-pushed the delete branch from 5d6998b to a56578f Compare July 15, 2016 09:36

fernandojsg merged commit 7eb84ad into aframevr:master Jul 15, 2016

fernandojsg deleted the delete branch July 15, 2016 09:38

fernandojsg added a commit to fernandojsg/aframe-inspector that referenced this pull request Jul 18, 2016

Merge pull request aframevr#197 from fernandojsg/delete

f271b9b

Added clone and remove entity to the Edit menu

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added clone and remove entity to the Edit menu #197

Added clone and remove entity to the Edit menu #197

Uh oh!

fernandojsg commented Jul 12, 2016 •

edited

Loading

ngokevin Jul 13, 2016

ngokevin commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

ngokevin commented Jul 13, 2016

dmarcos commented Jul 13, 2016

dmarcos commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

dmarcos commented Jul 13, 2016

dmarcos commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

dmarcos commented Jul 13, 2016 •

edited

Loading

fernandojsg commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

dmarcos commented Jul 14, 2016

dmarcos commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

dmarcos commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

ngokevin commented Jul 15, 2016

ngokevin Jul 15, 2016

fernandojsg Jul 15, 2016

ngokevin commented Jul 15, 2016

fernandojsg commented Jul 15, 2016

ngokevin commented Jul 15, 2016

fernandojsg commented Jul 15, 2016

ngokevin commented Jul 15, 2016

Labels

3 participants

Added clone and remove entity to the Edit menu #197

Added clone and remove entity to the Edit menu #197

Uh oh!

Conversation

fernandojsg commented Jul 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ngokevin Jul 13, 2016

Choose a reason for hiding this comment

ngokevin commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

ngokevin commented Jul 13, 2016

dmarcos commented Jul 13, 2016

dmarcos commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

dmarcos commented Jul 13, 2016

dmarcos commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

dmarcos commented Jul 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

fernandojsg commented Jul 13, 2016

fernandojsg commented Jul 13, 2016

dmarcos commented Jul 14, 2016

dmarcos commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

dmarcos commented Jul 14, 2016

fernandojsg commented Jul 14, 2016

ngokevin commented Jul 15, 2016

ngokevin Jul 15, 2016

Choose a reason for hiding this comment

fernandojsg Jul 15, 2016

Choose a reason for hiding this comment

ngokevin commented Jul 15, 2016

fernandojsg commented Jul 15, 2016

ngokevin commented Jul 15, 2016

fernandojsg commented Jul 15, 2016

ngokevin commented Jul 15, 2016

Labels

3 participants

fernandojsg commented Jul 12, 2016 •

edited

Loading

dmarcos commented Jul 13, 2016 •

edited

Loading