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

All groups are deleted after moving a group #1744

Closed
ybolognini opened this issue Aug 25, 2016 · 11 comments · Fixed by #1820 or #1954
Closed

All groups are deleted after moving a group #1744

ybolognini opened this issue Aug 25, 2016 · 11 comments · Fixed by #1820 or #1954
Assignees
Milestone

Comments

@ybolognini
Copy link
Member

To reproduce:

  • OSM theme on "desktop" example
  • Keeping mouse button down, start to move top group
  • Keeping mouse button down, move the group down and up a little
  • Move the group back to top place, release mouse button
  • Click on "remove" button, all groups are deleted!
@ybolognini ybolognini added this to the 2.1 milestone Aug 25, 2016
@ger-benjamin
Copy link
Member

To reproduce, you just have to select one top group, move it under another, place it anywhere, then delete it. All elements at is bottom will be deleted.

I think this arrives because we manipulate DOM in a ng-repeat. But I can't guarantee that.
If that's true, we can reproduce that on every ng-repeat with a move and delete function. We should do some tests.

@ger-benjamin
Copy link
Member

Yep, I can reproduce this error without the layertree. So that's come from ngeo-sortable or from AngularJS.

See a simple example here: https://ger-benjamin.github.io/ngeo/repeatError/examples/contribs/gmf/repeaterror.html

Code here (js): https://github.com/ger-benjamin/ngeo/blob/repeatError/contribs/gmf/examples/repeaterror.js
And there (html): https://github.com/ger-benjamin/ngeo/blob/repeatError/contribs/gmf/examples/repeaterror.html

@ger-benjamin
Copy link
Member

ger-benjamin commented Aug 29, 2016

So that's come from ngeo-sortable or from AngularJS.

Or from both... That because of how works angularJS and how we move the DOM That's the normal state:

selection_066

And that's after we have moved the DOM:

selection_067

The comment <!-- end ngRepeat: item in ctrl.items --> hasn't moved but that's the problem with AngularJS. We need to move it as well...

@sbrunner
Copy link
Member

I love angular ...

@ger-benjamin
Copy link
Member

ger-benjamin commented Sep 7, 2016

Ok... I try a lot of things but I can't to solve the problem with ngeo-sortable...

The only solution that I found is to use angular-ui-sortable instead.
This lib is a little bit more complex and heavy that our directive but do the same things and handle this issue: https://github.com/angular-ui/ui-sortable/blob/master/src/sortable.js#L335-L339

I try to manage this issue with the same idea that in angular-ui-sortable (save the state of the dom element then restore it if nothing has really moved) but i fail... I can't do any actionn on the elements after "restoring the DOM" perhaps because ngeo-sortable use goog and goog use goog uid..?

What should I do ?
Asking for help or implement angular-ui + sortable (from jquery-ui) and redo a few css ?

(code / example not up to date)

@pgiraud
Copy link
Contributor

pgiraud commented Sep 13, 2016

Note: this issue is not completely fixed. There are still issues with reordering that should be fixed with the layertree refactoring.

@sbrunner sbrunner reopened this Sep 13, 2016
@sbrunner
Copy link
Member

sbrunner commented Oct 5, 2016

Fixed with Layertree refactoring

@sbrunner sbrunner closed this as completed Oct 5, 2016
@pgiraud pgiraud reopened this Oct 5, 2016
@pgiraud
Copy link
Contributor

pgiraud commented Oct 5, 2016

This is unfortunately not completely fixed.

@sbrunner
Copy link
Member

sbrunner commented Oct 5, 2016

@pgiraud Can you provide a test case?

@pgiraud
Copy link
Contributor

pgiraud commented Oct 5, 2016

gmf_reorder_delete 2

@sbrunner
Copy link
Member

sbrunner commented Oct 5, 2016

Merci :-)

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