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

Fix re-ordering issues with Ember each #552

Merged
merged 2 commits into from
Aug 23, 2018

Conversation

sophypal
Copy link
Contributor

#PATCH#

The scenario that resulted in this weird behavior is when a condition involves a view layout where you have one field alongside other fields. Lets call the one field foo and call the others others. foo is a single field. others contain multiple fields. The way the view is laid out makes others use a key that represents the index while the foo field uses a key that matches the content. One is a number and the other is a string.

When conditions are involved, it'll toggle which field is displayed and the resulting keys being used change to match the new layout. We often end up with a scenario where there is a type mismatch with adjacent cells and 0 is used. This breaks the each functionality and instead of Ember re-ordering the DOM to match the rendered order of the component, it craps out and just appends it to the end. The fix involved forcing the keys to always be strings.

I would love to add tests for this but just testing this by hand was cumbersome so I'll leave it out for this PR.

CHANGELOG

  • Fixed condition re-ordering issue with Ember each in Ember 2.12 behaving weirdly with each when the comparison for keys involve type mismatches.

@sophypal sophypal requested a review from a team as a code owner August 22, 2018 22:36
@sophypal sophypal merged commit f4b3d21 into ciena-frost:master Aug 23, 2018
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.

2 participants