Skip to content

fix(common): A null value should remove the style on IE - #21679

Closed
vicb wants to merge 1 commit into
angular:masterfrom
vicb:0117-nullStyle
Closed

fix(common): A null value should remove the style on IE#21679
vicb wants to merge 1 commit into
angular:masterfrom
vicb:0117-nullStyle

Conversation

@vicb

@vicb vicb commented Jan 20, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

fixes #21064

Kudos to @ericmartinezr for a very detailed analysis of the issue in #21064

(The added test is failing on IE without the fix)

@vicb vicb added state: WIP area: common Issues related to APIs in the @angular/common package labels Jan 20, 2018
@vicb vicb added the target: patch This PR is targeted for the next patch release label Jan 20, 2018
@vicb vicb changed the title [WIP] fix(common): A null value should remove the style on IE fix(common): A null value should remove the style on IE Jan 20, 2018
@vicb vicb added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Jan 20, 2018
@mary-poppins

mary-poppins commented Jan 20, 2018

Copy link
Copy Markdown

You can preview 08db567 at https://pr21679-08db567.ngbuilds.io/.

alfaproject
alfaproject suggested changes Jan 20, 2018
View reviewed changes
Comment thread packages/common/test/directives/ng_style_spec.ts
}));

// https://github.com/angular/angular/issues/21064
it('should add and remove styles which names are not dash-cased', async(() => {

@alfaproject alfaproject Jan 20, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wrong description

@vicb vicb Jan 22, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why ?

@alfaproject alfaproject Jan 22, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could swear it was repeated. False alarm I guess o:

@vicb vicb added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 22, 2018
@mhevery mhevery closed this in 7d49443 Jan 22, 2018
mhevery pushed a commit that referenced this pull request Jan 22, 2018
@vicb @mhevery
fix(common): A null value should remove the style on IE (#21679)
c12ea3a 
fixes #21064

PR Close #21679
@vicb
vicb deleted the 0117-nullStyle branch January 29, 2018 19:25
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Sep 13, 2019

Copy link
Copy Markdown

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

2 more reviewers
@alfaproject alfaproject alfaproject requested changes
@IgorMinar IgorMinar IgorMinar approved these changes
Reviewers whose approvals may not affect merge requirements

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.

[core] ngStyle doesn't remove nulled styles in IE11

5 participants

@vicb @mary-poppins @alfaproject @IgorMinar @googlebot