Skip to content

Conversation

@gwak
Copy link

@gwak gwak commented Nov 29, 2024

This PR proposes a fix for the test case highlighted in the PR #599.

The changes removes the computed property on the firstItemPosition getter in sortable-group.ts modifier as it's not recomputed when the sortedItems array changes from the outside. If excessive recomputation of that getter gets expensive we could maybe use the more modern @cached decorator from @glimmer/tracking ?

@netlify
Copy link

netlify bot commented Nov 29, 2024

Deploy Preview for ember-sortable canceled.

Name Link
🔨 Latest commit 9f74129
🔍 Latest deploy log https://app.netlify.com/projects/ember-sortable/deploys/6863c86d0d5dcd0008f385bc

@gwak
Copy link
Author

gwak commented Feb 17, 2025

Hi @cyril-sf, can any maintainer look into this issue and related PR ? We're currently using our fork in our main production application, but would prefer not to...

@NullVoxPopuli
Copy link
Contributor

I'd feel more comfortable if this PR provided a test that failed before the changes here, and then passes afterwards <3

@NullVoxPopuli
Copy link
Contributor

The changes to the test app feel a smidge excessive? Why'd every example have to change? Can something more targeted happen instead?

@NullVoxPopuli
Copy link
Contributor

Also, biggest apologies for not commenting sooner. I don't know how i missed this PR <3

@gwak gwak force-pushed the fix/non-refreshed-computed-property branch from 3160b76 to 27a34c7 Compare March 5, 2025 14:53
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.

3 participants