Skip to content

Conversation

@belgamo
Copy link

@belgamo belgamo commented Dec 29, 2019

When I needed to use the laneSortFunction I notice that it's work fine in a single lane as showed in the story example. But there's a bug when I have more than one lane. The react-trello gets confused with the new indexes and consequently move wrong cards on dragging and dropping.

I fixed the issue by reordering the cards before they were added on the lane.

@belgamo belgamo requested a review from dapi as a code owner December 29, 2019 12:48
@belgamo belgamo force-pushed the fix/sort-functionality branch from a1d56be to ec2548d Compare December 29, 2019 12:53
@belgamo belgamo force-pushed the fix/sort-functionality branch from ec2548d to e762557 Compare December 29, 2019 13:06
@belgamo belgamo changed the title feat: Sort cards before state update Sort cards before state update Dec 29, 2019
belgamo and others added 2 commits December 30, 2019 08:52
Co-Authored-By: Bruno Castro <bruno.castro@codeminer42.com>
Co-Authored-By: Bruno Castro <bruno.castro@codeminer42.com>
@belgamo belgamo force-pushed the fix/sort-functionality branch from 61f7ce8 to 494cbf1 Compare December 30, 2019 12:17
@belgamo belgamo force-pushed the fix/sort-functionality branch from 494cbf1 to 224ad79 Compare December 30, 2019 13:37
Copy link
Collaborator

@dapi dapi left a comment

Choose a reason for hiding this comment

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

@belgamo hi!

Thank you very much for such detailed PR!

I pulled this branch and have played with some stories and found that basic scenario with card dragging does not work as it expected.

Here is sample - http://g.recordit.co/DjFi6MNr34.gif

@dapi
Copy link
Collaborator

dapi commented Feb 24, 2020

Probably we need to disable card dragging when laneSortFunction is used? What do you think?

if (lane.id === laneId) {
if (index !== undefined) {
return update(lane, {cards: {$splice: [[index, 0, ...newCards]]}})
let cardsToUpdate = [...lane.cards, ...newCards]
Copy link
Author

Choose a reason for hiding this comment

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

@dapi Hi!

I think your comment #337 (comment) makes sense! However, the problem occurs because this line changes the original cards and this influences their order even when the laneSortFunction is not used. I suggest the code bellow:

if (laneSortFunction) {
  let cardsToUpdate = [...lane.cards, ...newCards]
  cardsToUpdate.sort(laneSortFunction)

  return update(lane, {cards: {$set: cardsToUpdate}})
}

return update(lane, {cards: {$splice: [[index, 0, ...newCards]]}})

What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @belgamo!

What is you use case to sort cards? Do you save these cards to backend? Does it sort on backend also?

Pay attention to there are event bus and we need to emit MOVE_CARD event to every moved card if it is resorted.

I think the better way to give possibility to customise appendCardsToLane function using dependency injection.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One more thought. Can we use event MOVE_CARD to catch this moment and resort cards as we want?

@belgamo belgamo requested a review from dapi March 21, 2020 13:18
@orangecoding
Copy link
Contributor

Hey guys. what's the status on this? I just spend like 2 hours to track this problem down until I found that somebody already did it... 😱
Any eta when we can expect an update?

@LucasBadico
Copy link

Guys this solution is not working. Seen the same thing. I have just merged with the v2.2.8 and is the same as before.

Copy link
Collaborator

@dapi dapi left a comment

Choose a reason for hiding this comment

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

either respect event bus and emit event for resorted cards or use dependency injection to customise appendCardsToLane

@belgamo belgamo closed this Jan 13, 2023
@belgamo belgamo deleted the fix/sort-functionality branch January 13, 2023 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants