Skip to content

fix(common): properly take className changes into account #21937

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fix(common): properly take className changes into account #21937

wants to merge 2 commits into from

Conversation

pkozlowski-opensource
Copy link
Member

Fixes #21932

@pkozlowski-opensource pkozlowski-opensource added target: patch This PR is targeted for the next patch release action: review The PR is still awaiting reviews from at least one requested reviewer area: common Issues related to APIs in the @angular/common package labels Jan 31, 2018
@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Jan 31, 2018
Closed
@vicb vicb added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Feb 20, 2018
@vicb
Copy link
Contributor

vicb commented Feb 20, 2018 •
edited
Loading

@pkozlowski-opensource When you are back from vacation could you please add comments to this code ?
Each time I look at it I have to track down to where isCleanup is set to understand it.

Also it would be worth explaining the change of behavior in the commit message.

Thanks

@pkozlowski-opensource pkozlowski-opensource removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Feb 27, 2018
@pkozlowski-opensource
Copy link
Member Author

@vicb refactored the code so (in my opinion) it is easier to follow. Added comments as well. Please have another look.

@pkozlowski-opensource pkozlowski-opensource force-pushed the ng_class_and_class_binding branch from e733090 to 1e9941c Compare April 4, 2018 12:54
@vicb
Copy link
Contributor

vicb commented Apr 4, 2018

http://test/OCL:191595536:BASE:191595560:1522856502593:4911ef2d

@pkozlowski-opensource pkozlowski-opensource removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 4, 2018
@vicb vicb added the action: merge The PR is ready for merge by the caretaker label Apr 4, 2018
@vicb
Copy link
Contributor

vicb commented Apr 4, 2018

Thanks for the updates @pkozlowski-opensource. The code looks nice and easy to follow!

@alxhub alxhub closed this in 4a42669 Apr 4, 2018
alxhub pushed a commit that referenced this pull request Apr 4, 2018
@pkozlowski-opensource @alxhub
refactor(common): simplify NgClass code, add comments (#21937)
aaefd51 
PR Close #21937
alxhub pushed a commit that referenced this pull request Apr 4, 2018
@pkozlowski-opensource @alxhub
fix(common): properly take className changes into account (#21937)
54e9108 
Fixes #21932

PR Close #21937
alxhub pushed a commit that referenced this pull request Apr 4, 2018
@pkozlowski-opensource @alxhub
refactor(common): simplify NgClass code, add comments (#21937)
3900c36 
PR Close #21937
@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 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vicb vicb vicb approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ngClass remove the class only if the object's value is null
3 participants
@pkozlowski-opensource @vicb @googlebot