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

[docs] Improve wording for animate docs, refs #6807 #6813

Merged
merged 2 commits into from
Oct 20, 2021

Conversation

Prinzhorn
Copy link
Contributor

Quoting #6807

Usually when you add or remove an item from a list, you don't consider this a "re-order". Yes, technically all elements after the new/removed element change their position within the list, but the order is exactly the same. If you stand in line for coffee and the person in front of you gets a call and leaves you wouldn't say the order of the line changed. I'm not a native speaker though shrug

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].

Quoting sveltejs#6807

> Usually when you add or remove an item from a list, you don't consider this a "re-order". Yes, technically all elements after the new/removed element change their position within the list, but the order is exactly the same. If you stand in line for coffee and the person in front of you gets a call and leaves you wouldn't say the order of the line changed. I'm not a native speaker though shrug
@@ -1132,7 +1132,7 @@ DOMRect {

---

An animation is triggered when the contents of a [keyed each block](docs#each) are re-ordered. Animations do not run when an element is removed, only when the each block's data is reordered. Animate directives must be on an element that is an *immediate* child of a keyed each block.
An animation is triggered when an element inside a [keyed each block](docs#each) changes its position. Animations do not run for newly added or removed elements. They only run when the data of an existing element changes position within the each block's data. Animate directives must be on an element that is an *immediate* child of a keyed each block.
Copy link
Member

Choose a reason for hiding this comment

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

I have to say I'm not a big fan of this change. The passive voice sounds odd, it's more verbose, and it doesn't mean anything different to me

Copy link
Member

Choose a reason for hiding this comment

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

Maybe (versus the original one) "Animations do not run when an element is added or removed" (adding the bit about "added or")? I agree that the suggested changes here don't bring much that's new.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't mean anything different to me
I agree that the suggested changes here don't bring much that's new.

My main point is to remove the "are re-ordered" part. Because that threw me off and I assumed animate only runs when the order of the elements changes. Maybe this is a language barrier, but to me "order" (related to the term "sort order") is defined by the comparator function or some intrinsic property of the list. If I remove an element from the list then nothing is re-ordered. The order does not change.

But like I said, I'm not a native speaker and the only time I come across the term "order" is in a programming/cs context (but I'm aware that there are other orders, such as in a military context or when I order something online, that are entirely unrelated)

The docs say:

Animations do not run when an element is removed

But they do. The current wording uses the plural form "Animations" when in reality each animation has to be looked at in isolation (because the logic only looks if the index of a given element has changed, ignoring anything around it). Because yes, when an element is removed the animations (plural) do run, but not the animation (singular) of the removed element. Only those animations of elements that come after run.

I assume it is worded this way because the original author knew about transitions and maybe it is worded this way in comparison to transitions (which run when elements are created/removed). But without the transition knowledge the animation paragraph is misleading.

Copy link
Member

Choose a reason for hiding this comment

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

How about something like this?

Animations do not run when an element is added or removed, only when the index of a data item within the each block is changed

@Prinzhorn
Copy link
Contributor Author

I simplified it via @benmccann's suggestion. It's now clear that it doesn't run for added elements either, not just for removed elements. And it makes clear the the index of an existing item needs to change for it to trigger an animation.

@benmccann benmccann merged commit 6f8a6fe into sveltejs:master Oct 20, 2021
@Prinzhorn Prinzhorn deleted the patch-1 branch October 20, 2021 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants