Skip to content

fix(compiler): handle case-sensitive CSS custom properties#41380

Closed
crisbeto wants to merge 2 commits intoangular:masterfrom
crisbeto:41364/css-variables-case-sensitive
Closed

fix(compiler): handle case-sensitive CSS custom properties#41380
crisbeto wants to merge 2 commits intoangular:masterfrom
crisbeto:41364/css-variables-case-sensitive

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Mar 30, 2021

Currently we normalize all CSS property names in the StylingBuilder which breaks custom properties, because they're case-sensitive. These changes add a check so that custom properties aren't normalized.

Fixes #41364.

@crisbeto
fix(compiler): handle case-sensitive CSS custom properties
21c557e 
Currently we normalize all CSS property names in the `StylingBuilder` which breaks custom properties, because they're case-sensitive. These changes add a check so that custom properties aren't normalized.

Fixes angular#41364.
@google-cla google-cla bot added the cla: yes label Mar 30, 2021
@crisbeto crisbeto marked this pull request as ready for review March 30, 2021 15:29
@pullapprove pullapprove bot requested a review from AndrewKushnir March 30, 2021 15:29
@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: compiler Issues related to `ngc`, Angular's template compiler target: major This PR is targeted for the next major release labels Mar 30, 2021
@ngbot ngbot bot modified the milestone: Backlog Mar 30, 2021
AndrewKushnir
AndrewKushnir approved these changes Mar 30, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @crisbeto 👍 I've just left a few minor comments.

I've also noticed that you've applied "target: major" label, just curious if you see any potential risks with this change that might be worth mentioning in commit message or here in PR description?

Thank you.

@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 30, 2021
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Mar 30, 2021

Presubmit.

@crisbeto
Copy link
Copy Markdown
Member Author

crisbeto commented Mar 30, 2021

Regarding the targeting, I was just confused whether targeting patch now would go out in the first patch after v12 or now. I don't think it's particularly risky.

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Mar 30, 2021
@AndrewKushnir AndrewKushnir added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: presubmit The PR is in need of a google3 presubmit labels Mar 30, 2021
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Mar 30, 2021

@crisbeto FYI presubmits are successful for the changes in this PR. I'm adding the "cleanup" label for now to see if you want to change the "target" label (if not, just remove the "cleanup" label). Thank you.

@crisbeto crisbeto added target: patch This PR is targeted for the next patch release and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: major This PR is targeted for the next major release labels Mar 30, 2021
alxhub pushed a commit that referenced this pull request Mar 30, 2021
@crisbeto @alxhub
fix(compiler): handle case-sensitive CSS custom properties (#41380)
f9da272 
Currently we normalize all CSS property names in the `StylingBuilder` which breaks custom properties, because they're case-sensitive. These changes add a check so that custom properties aren't normalized.

Fixes #41364.

PR Close #41380
@alxhub alxhub closed this in e112e32 Mar 30, 2021
TeriGlover pushed a commit to TeriGlover/angular that referenced this pull request Apr 5, 2021
@crisbeto @TeriGlover
fix(compiler): handle case-sensitive CSS custom properties (angular#4…
c3ea7dd 
…1380)

Currently we normalize all CSS property names in the `StylingBuilder` which breaks custom properties, because they're case-sensitive. These changes add a check so that custom properties aren't normalized.

Fixes angular#41364.

PR Close angular#41380
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Apr 30, 2021

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 Apr 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Custom CSS Properties case sensitivity not respected with HostBinding

2 participants

@crisbeto @AndrewKushnir