Skip to content

[#515, #524] nodrop, clone, and dirty-checking fixes#525

Merged
Voles merged 2 commits into
angular-ui-tree:masterfrom
ppuleo:515-524-nodrop_clone_fixes
Aug 5, 2015
Merged

[#515, #524] nodrop, clone, and dirty-checking fixes#525
Voles merged 2 commits into
angular-ui-tree:masterfrom
ppuleo:515-524-nodrop_clone_fixes

Conversation

@ppuleo

@ppuleo ppuleo commented Jul 27, 2015

Copy link
Copy Markdown
Contributor
  • Use angular.copy when cloning nodes
  • Simplify nodrop, clone, and isDirty checking in $uiTreeHelper dragInfo
  • Update $uiTreeHelper dragInfo.apply to allow null parent.$nodeScope for single-level trees

@ppuleo

ppuleo commented Jul 28, 2015

Copy link
Copy Markdown
Contributor Author

Hmm, looks like the tests are starting in the middle of the build. I think runSequence might be a better than gulp.start for the build task.

@Voles

Voles commented Jul 28, 2015

Copy link
Copy Markdown
Member

Thanks for this, @ppuleo! CircleCI isn't well configured so don't mind the failing build. As long as Travis succeeds.

Can you elaborate your improvements, please? Do you need these for a project you're working on?

@ppuleo

ppuleo commented Jul 29, 2015

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing @Voles! And thanks for maintaining this project. This PR addresses two issues:

  1. Use angular.copy when cloning a node to a different tree. I described the issue in Clone appears to create object references rather than new objects #524, and added an example.
  2. Simplify the data-nodrop-enabled and drop-in-place cases in the $uiTreeHelper service dragInfo.apply method. The logic in there was getting hard to follow and there was already a method defined to check for a node dropped in its original place (isDirty). Based on the conversation in Multiple issues: nodrop, clone and related functionality #515, this PR enforces cancellation of the drop if data-nodrop-enabled is added to either the destination ui-tree or the immediate ui-tree-nodes parent element.

I'm using angular-ui-tree in a project now so I'll try to contribute fixes for any bugs I find. So far, it generally works well.

@Voles

Voles commented Jul 31, 2015

Copy link
Copy Markdown
Member

@ppuleo thanks for your detailed explanation! I've tested your changes, LGTM!
Because of the v2.7.0 release, there are conflicts with your branch.

If you resolve these conflicts I'll merge your changes! Sorry for the inconvenience...

@ppuleo

ppuleo commented Aug 1, 2015

Copy link
Copy Markdown
Contributor Author

@Voles Sure, no problem. I should probably add an example as well. I'll update and comment again when this is ready to review.

@Voles

Voles commented Aug 2, 2015

Copy link
Copy Markdown
Member

@ppuleo great! Feel free to add the examples as a separate PR. Maybe I can help you with this as well.

@Voles

Voles commented Aug 2, 2015

Copy link
Copy Markdown
Member

I'd like to suggest to update the current connected-trees example page. By adding some information about cloning the node and using input-fields for the node name, things should be clear enough. WDYT?

Example page: http://angular-ui-tree.github.io/angular-ui-tree/#/connected-trees

@ppuleo
ppuleo force-pushed the 515-524-nodrop_clone_fixes branch from ce43796 to 7d2d351 Compare August 3, 2015 07:03
ppuleo added 2 commits August 3, 2015 00:07
@ppuleo
[angular-ui-tree#515, angular-ui-tree#524] nodrop, clone, and dirty-c…
95daa8d 
…hecking fixes

- Use angular.copy when cloning nodes
- Simplify nodrop, clone, and isDirty checking in $uiTreeHelper dragInfo
- Update $uiTreeHelper dragInfo.apply to allow null parent.$nodeScope for single-level trees
@ppuleo
[angular-ui-tree#515, angular-ui-tree#524] no-drop and clone examples
4da9833 
- Add an example for data-nodrop-enabled on the ui-tree and on an individual node
- Add an example for cloning
@ppuleo
ppuleo force-pushed the 515-524-nodrop_clone_fixes branch from 7d2d351 to 4da9833 Compare August 3, 2015 07:21
@ppuleo

ppuleo commented Aug 3, 2015

Copy link
Copy Markdown
Contributor Author

@Voles ok, I've added examples for data-nodrop-enabled and cloning between connected trees so I think this is ready for review. Thanks!

@vitorenesduarte

vitorenesduarte commented Aug 4, 2015

Copy link
Copy Markdown

+1

Voles pushed a commit that referenced this pull request Aug 5, 2015
Merge pull request #525 from ppuleo/515-524-nodrop_clone_fixes
a855c25 
[#515, #524] nodrop, clone, and dirty-checking fixes
@Voles
Voles merged commit a855c25 into angular-ui-tree:master Aug 5, 2015
@Voles

Voles commented Aug 5, 2015

Copy link
Copy Markdown
Member

Merged. Thanks very much, @ppuleo! The examples are very useful for other developers!

@ppuleo
ppuleo deleted the 515-524-nodrop_clone_fixes branch August 11, 2015 22:16
@ppuleo ppuleo mentioned this pull request Aug 11, 2015
Closed
@wwsno wwsno mentioned this pull request Oct 22, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ppuleo @Voles @vitorenesduarte