Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(ngAnimate): add animating any scope's variable change section #16582

Closed
wants to merge 1 commit into from

Conversation

FRSgit
Copy link
Contributor

@FRSgit FRSgit commented May 27, 2018

Add a section which covers use case when user's need to animate upon every variable's value change.

Refers #16561

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Added new section to ngAnimate's module page.

What is the current behavior? (You can also link to an open issue here)
That's an effect of discussion which took place under #16561

What is the new behavior (if this is a feature change)?
No, just docs.

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Fix/Feature: Docs have been added/updated
  • Fix/Feature: Tests have been added; existing tests pass

Other information:

* ```
*
* ### Animating any scope's variable change
Copy link
Member

Choose a reason for hiding this comment

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

I think this title isn't very clear. It makes it sound as if emphasis is put on any scope.
AFAICT, what you are trying to say is "animating between arbitrary values" (i.e. not between two states but between any number of (unknown) states; similar to Angular's * => * (from any state to any state)).

Not sure what's a good heading though 😳

Copy link
Contributor Author

@FRSgit FRSgit Jun 1, 2018

Choose a reason for hiding this comment

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

Maybe something like:

  • Animating on any value change of a directive,
  • Animating between subsequent value changes,
  • or as you've written it Animating between arbitrary values.

* ```
*
* ### Animating any scope's variable change
*
* Unless possible with more proper ["animation aware" directive](#directive-support), that specific use case can always be covered with {@link ngAnimate.directive:ngAnimateSwap} as can be seen in {@link ngAnimate.directive:ngAnimateSwap#examples this example}.
Copy link
Member

Choose a reason for hiding this comment

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

I would expand here on what type of usecase ngAnimateSwap helps support. I.e. that you can animate between multiple (>2) different states (variable values), whose values don't need to be known or referenced in the CSS styles.

BTW, it is better for readability and "review-ability" if lines are wrapped at 100 chars.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added it in newest commits.

@gkalpak gkalpak self-assigned this May 28, 2018
@FRSgit FRSgit force-pushed the animate-swap-docs branch from 357b0d2 to 4df3ba0 Compare June 1, 2018 02:23
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Getting there 😃

* ```
*
* ### Animating between subsequent value changes
*
* Sometimes you need to animate between multiple (2 and more), different expression states, whose values
Copy link
Member

Choose a reason for hiding this comment

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

2 and more --> more than 2

Copy link
Contributor Author

@FRSgit FRSgit Jun 1, 2018

Choose a reason for hiding this comment

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

Actually it will work also for animating between just 2 states, so IMO it should be 2 or more

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. Since you can't animate between...one state, let's drop the multiple (2 and more) part.

Sometimes you need to animate between different expression states whose values don't necessary need to be known or referenced in CSS styles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

💯

* Note that {@link ngAnimate.directive:ngAnimateSwap} is a *structural directive*, which means it
* creates a new instance of the element (including any other/child directives it may have) and
* links it to a new scope every time *swap* happens. In some cases this might not be desirable
* (e.g. for performance reasons, or when wish to retain internal state on the original
Copy link
Member

Choose a reason for hiding this comment

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

when wish --> when you wish

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👌

* creates a new instance of the element (including any other/child directives it may have) and
* links it to a new scope every time *swap* happens. In some cases this might not be desirable
* (e.g. for performance reasons, or when wish to retain internal state on the original
* element's instance).
Copy link
Member

Choose a reason for hiding this comment

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

element's instance --> element instance (maybe?)

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 seems my english teacher was too much into adding 's always when you want to write about possession :D

Copy link
Member

Choose a reason for hiding this comment

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

Well, I'm not a native speaker either, so I could be wrong (😱).
But, I don't think it is about possesion here (the instance does not belong to the element).

*
* Sometimes you need to animate between multiple (2 and more), different expression states, whose values
* don't necessary need to be known or referenced in CSS styles.
* Unless possible with more proper ["animation aware" directive](#directive-support), that specific
Copy link
Member

Choose a reason for hiding this comment

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

I would remove "more proper". (What is "proper" anyway? Plus ngAnimateSwap is no less "proper" than other "animation aware" directives 😁)
So, I would change "with more proper" to "with another".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I had in mind was that sometimes "other" directive can be non-structural, and because of this is often more preferable, ergo better when it comes to performance 😄 I'll change that.

Copy link
Member

Choose a reason for hiding this comment

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

Some times a structural directive might be what you want (because for example you want to discard internal state 😉). It depends on the usecase. (It is not that structural directives are objectively inferior to non-structural ones.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True

* ```
*
* ### Animating between subsequent value changes
Copy link
Member

Choose a reason for hiding this comment

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

Better thatn before, but I think "arbitrary" instead of "subsequent" would be preferable (because you always want to animate "subsequent changes").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you think so - sorry, I cannot figure out what will be meaning of arbitrary in such case. IT's like 'random'/'undetermined'/'any'?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I had this meaning in mind:

Mathematics. undetermined; not assigned a specific value:
an arbitrary constant.

Maybe "unknown"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this. Maybe it should be just Animating between value changes?

Copy link
Member

Choose a reason for hiding this comment

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

Let's go with that. (It has its own issue, e.g. being too generic, but it is better than what we (don't) have now 😃)

@FRSgit FRSgit force-pushed the animate-swap-docs branch from 4df3ba0 to b5644d8 Compare June 4, 2018 21:45
Add a section which covers use case when user's need to animate upon every variable's value change.

Refers angular#16561
@FRSgit FRSgit force-pushed the animate-swap-docs branch from b5644d8 to 41f19c1 Compare June 4, 2018 21:48
@FRSgit
Copy link
Contributor Author

FRSgit commented Jun 4, 2018

@gkalpak done!

@gkalpak gkalpak closed this in 0ed3643 Jun 5, 2018
gkalpak pushed a commit that referenced this pull request Jun 5, 2018
Add a section which covers use case when users need to animate upon
a variable's value changes (not between two states).

Refers #16561

Closes #16582
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants