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

docs(ngShow/ngHide): add note about flicker when toggling elements #16489

Merged
merged 1 commit into from
Mar 14, 2018

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Mar 13, 2018

Related to #14015

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?

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:

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.

A few minor suggetions.
LGTM otherwise.

*
* ### Flickering when using ngShow to toggle between elements
*
* When using {@link ngShow} and / or {@link ngHide} to toggle between elements , it can
Copy link
Member

Choose a reason for hiding this comment

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

Extra whitespace before comma.

* ### Flickering when using ngShow to toggle between elements
*
* When using {@link ngShow} and / or {@link ngHide} to toggle between elements , it can
* happen that both the element to show and the element to hide are visible at the same time.
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 mention that this only happens for a brief moment; it is not a permanent state.
It might also be worth mentioning that it affects some browsers (*cough* IE *cough*) more than others.

* There are several way to mitigate this problem:
*
* - {@link guide/animations#how-to-selectively-enable-disable-and-skip-animations Disable animations on the affected elements}.
* - Use {@link ngIf} instead of {@link ngShow} / {@link ngHide}.
Copy link
Member

Choose a reason for hiding this comment

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

Actually, if you are toggling, ngSwitch is more appropriate 😁

* - {@link guide/animations#how-to-selectively-enable-disable-and-skip-animations Disable animations on the affected elements}.
* - Use {@link ngIf} instead of {@link ngShow} / {@link ngHide}.
* - Use the special CSS selector `ng-hide.ng-hide-animate` to set `{display: none}` or similar on the affected elements .
* - Define an animation on the affected elements.
Copy link
Member

Choose a reason for hiding this comment

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

😂

* This usually happens when the {@link ngAnimate ngAnimate module} is included, but no actual animations
* are defined for {@link ngShow} / {@link ngHide}.
*
* There are several way to mitigate this problem:
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 using ng-class="{'ng-hide': expression}?

*
* - {@link guide/animations#how-to-selectively-enable-disable-and-skip-animations Disable animations on the affected elements}.
* - Use {@link ngIf} instead of {@link ngShow} / {@link ngHide}.
* - Use the special CSS selector `ng-hide.ng-hide-animate` to set `{display: none}` or similar on the affected elements .
Copy link
Member

Choose a reason for hiding this comment

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

Extra whitespace before the period.

@Narretz Narretz force-pushed the docs-ngshow-flicker branch 2 times, most recently from 707d21f to 4ad23ca Compare March 14, 2018 20:57
@Narretz Narretz merged commit 98e0e04 into angular:master Mar 14, 2018
@Narretz Narretz deleted the docs-ngshow-flicker branch March 14, 2018 21:35
gkalpak pushed a commit that referenced this pull request Mar 16, 2018
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