Skip to content

Drag&drop list sorting is not updated correctly on the first try. #351

Closed
href opened this Issue Nov 13, 2013 · 5 comments

2 participants

@href
Plone Collective member
href commented Nov 13, 2013

The contents of list styles may be sorted by dragging and dropping.

The problem is that the first time always fails. This means that if I drag an item to the top of the list it seems to return to its previous position. If I repeat the action the sorting works.

The position is actually stored correctly, because after the first drag I can also do a page refresh, which shows the expected sort order.

So it seems like the ajax request that will immediatly show the result of the reordering shows stale information.

@hvelarde
Plone Collective member

you have to drop the tile when the new position change to green; of course it could be enhanced, but is not a bug.

@hvelarde hvelarde closed this Nov 13, 2013
@href
Plone Collective member
href commented Nov 14, 2013

The issue actually occurs even if the position changes to green. I'll investigate further and try to find the cause.

@href
Plone Collective member
href commented Nov 14, 2013

This is what happens:

  1. @@updatelisttilecontent is called in cover.js:30 when the drop is finished
  2. replace_with_objects is called in content.py:266 with the new order
  3. replace_with_objects in list.py:129 gets a new instance of ITileDataManager to set the new order
  4. The list is rendererd through the results function in list.py:80 using self.data. Since self.data uses a cache in plone.tiles.tile the order is not up to date at this point.
  5. The resulting html replaces the current list in cover.js:69

I'm not sure which is the best way to fix this. I don't know why list.py uses self.data at some times, but ITileDataManager at other times. If everything went through self.data it would be fine (though I don't see where the changes are written back in this case). If everything went through ITileDataManger it should also work, but then there's no caching.

Lastly, the cover.js could just ignore the resulting html because the dom is already up to date after the drop. But that seems like treading the symptom, not the cause.

What do you think? I don't mind getting my hands dirty and fixing this myself, pull request and all. But I do need some guidance with me being new to this codebase.

@href href reopened this Nov 14, 2013
@hvelarde
Plone Collective member

please help us fixing it, but first try to add some lines to the Robot Framework tests to demonstrate the reordering functionality and the bug if possible.

open a new branch and feel free to fix whatever you need to; the code of that tile is old and is plenty of insane stuff.

we haven't had the time to look at it very closely.

@href
Plone Collective member
href commented Nov 14, 2013

Got it, thanks.

@href href added a commit that referenced this issue Nov 18, 2013
@href href Adds failing test for issue #351 54984e1
@hvelarde hvelarde closed this in #356 Nov 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.