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

Class directive not applied correctly for ngFor and ChangeDetectionStrategy.OnPush #206

Closed
dfmartin opened this issue Mar 6, 2017 · 4 comments · Fixed by #228
Closed
Assignees
Milestone

Comments

@dfmartin
Copy link

dfmartin commented Mar 6, 2017

I am trying to use ChangeDetectionStrategy.OnPush along with an ngFor, but have found that the class/ngClass directive does not work when using OnPush. What I am finding is that the appropriate class is being applied to only the last item in the list.

This occurs only on the initial render. If the screen is resized to cause a change then everything works from that point forward. I have tried ChangeDetectorRef.markForCheck() and seen no change. One thing that causes some head scratching is that the class is applied to the last item so it's obvious the class directive is firing correctly somewhere.

I have put up a plunker (appropriated from a demo for side nav). If you look at the file src/app/responsive-sidenav.ts . There is a line in the meta that can be commented out to see the "correct" behavior.

http://plnkr.co/edit/zalFNqqiznHUyg33FJII?p=preview

@ThomasBurleson ThomasBurleson self-assigned this Mar 6, 2017
@ThomasBurleson ThomasBurleson modified the milestones: v2.0.0-rc.2, v2.0.0-rc.3 Mar 6, 2017
@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Mar 16, 2017

This will be fixed with PR https://github.com/angular/flex-layout/pull/223/files.

Also here is a working Plunkr version (with the latest flex-layout code).

  • Note the html <div class='item' ngClass.gt-xs='item showOnGtXs'>
  • Note the css
.item.showOnGtXs {
  color: #932e2e;
  background-color: #DDECFE
}

Here are some docs explaining how to add/remove/merge classes: https://github.com/angular/flex-layout/wiki/ngClass-API#responsive-features

Note: the initial page load issue is still present.

@ThomasBurleson ThomasBurleson modified the milestones: v2.0.0-rc.2, v2.0.0-rc.3 Mar 16, 2017
@ThomasBurleson ThomasBurleson added the has pr A PR has been created to address this issue label Mar 16, 2017
ThomasBurleson added a commit that referenced this issue Mar 16, 2017
*  add missing static `ngClass` and `class`  selectors
*  add tests to demonstrate fallbacks to static selector values

Fixes #206.
@ThomasBurleson ThomasBurleson removed the has pr A PR has been created to address this issue label Mar 16, 2017
@ThomasBurleson ThomasBurleson modified the milestones: v2.0.0-rc.3, v2.0.0-rc.2 Mar 16, 2017
@ThomasBurleson
Copy link
Contributor

Reopen since the ngFor issue is still present.

@ThomasBurleson
Copy link
Contributor

ThomasBurleson commented Mar 19, 2017

@dfmartin - see working Plunkr https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

Note: you do not need to use this.cdr.markForCheck(); in the $media.subscribe handler.

uses local snapshot build of the PR flex-layout.umd.js

ThomasBurleson added a commit that referenced this issue Mar 27, 2017
* ngClass now properly differentiates between `ngClass` and `class`
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes

@see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206.
ThomasBurleson added a commit that referenced this issue Mar 27, 2017
…rategy.OnPush strategies

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206.
ThomasBurleson added a commit that referenced this issue Mar 27, 2017
…rategy.OnPush strategies

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206.
ThomasBurleson added a commit that referenced this issue Mar 27, 2017
…rategy.OnPush strategies

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206, fixes #215.
ThomasBurleson added a commit that referenced this issue Mar 28, 2017
…rategy.OnPush strategies

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206, fixes #215.
ThomasBurleson added a commit that referenced this issue Mar 28, 2017
…rategy.OnPush strategies

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.
* fix class lt-<xx> selectors

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206, fixes #215.
ThomasBurleson added a commit that referenced this issue Mar 28, 2017
…rategy.OnPush strategies

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.
* fix class lt-<xx> selectors

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206, fixes #215.
tinayuangao pushed a commit that referenced this issue Mar 29, 2017
…rategy.OnPush strategies (#228)

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.
* fix class lt-<xx> selectors

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes #206, fixes #215.
karlhaas pushed a commit to karlhaas/flex-layout that referenced this issue May 3, 2017
*  add missing static `ngClass` and `class`  selectors
*  add tests to demonstrate fallbacks to static selector values

Fixes angular#206.
karlhaas pushed a commit to karlhaas/flex-layout that referenced this issue May 3, 2017
…rategy.OnPush strategies (angular#228)

* fix(ngClass):  properly differentiate between `ngClass` and `class` api usages
  *  `class.<xxx>` are destructive overrides (as seen in `NgClass::klass`)
  *  `ngClass.<xxx>` use iterables to merge/enable/disable 0...n classes
* fix(ngClass,ngStyle): support ChangeDetectionStrategy.OnPush
  * Add support for both **OnPush** change detection strategies and for @input changes.
* fix class lt-<xx> selectors

>  @see https://plnkr.co/edit/jCICQ1GXFnzFqFTFlXwh?p=preview

fixes angular#206, fixes angular#215.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants