fix(compiler): fix emulated css functional selectors scoping#45807
fix(compiler): fix emulated css functional selectors scoping#45807dario-piotrowicz wants to merge 1 commit intoangular:mainangular/angular:mainfrom dario-piotrowicz:fix-css-functional-pseudo-classesdario-piotrowicz/angular:fix-css-functional-pseudo-classesCopy head branch name to clipboard
Conversation
packages/compiler/src/shadow_css.ts
Outdated
There was a problem hiding this comment.
PS, I could actually get the placeholders not to include the parenthesis so that here we would have:
':is(%SELECTORFNARGS%):not(%SELECTORFNARGS%):not(%SELECTORFNARGS%) {}'
but I opted for not doing that as it would require extra logic to be run and it also doesn't really seem to add any actual value to the solution
ed66073 to
5421982
Compare
5421982 to
bc82d99
Compare
|
Thanks Dario, is this ready for review? |
Yes 馃槂 |
|
Mind rebasing and reformatting for lint? I will run a presubmit and find a reviewer |
bc82d99 to
c0de124
Compare
done, thank you very much! 馃槃 馃檹 |
|
@AndrewKushnir @JoostK What do you guys this of this? It looks OK to me, but I'm admittedly not an expert in this area |
c0de124 to
e261835
Compare
|
@dario-piotrowicz thanks for creating this PR. We've reviewed the changes and there are some potential risks related to the extra processing. We'd need to dedicate additional time to dig into the code more, investigate potential implications and work with Google's codebase (if needed). Since the original issue doesn't have a strong signal from the community, we plan to put this PR on hold (close it for now). We'll come back to this fix if there is a stronger signal from the community in the original bug report. Thank you. |
馃槩 Also even if it doesn't have a strong signal from the community, it does look to me like this is basic functionality which should indeed work (like in the reproduction in the issue itself, there is no clear indication that this sort of thing doesn't work, it simply generates some css which is just plain wrong). Anyways if it introduces some risks and you don't feel comfortable merging it, I understand, I hope that maybe it can be revisited at some point. Please let me know if there is anything I can do to help here 馃檪 |
|
Discussed offline: we'll run a TGP for this change and decide how to proceed based on the outcome. |
e261835 to
53d83f8
Compare
b6bbe51 to
f5f2d91
Compare
currently in emulated view encapsulation if a selector contains a functional pseudo-class selector (such as :is() for example) and contains different spaces and commas inside the parenthesis such characters are handle as if there where part of a normal selector thus causing various parsing issues, solve this issue by escaping/unescaping the content of such selectors Basically if a selector is: `:is(.foo, .bar):not(.baz)` escape it to `:is_PLACEHOLDER_:not__PLACEHOLDER__`, handle/scope it as normal and then revert back the placeholders to the original values resolves angular#45686
6244f20 to
0f9c0fc
Compare
|
Hi, are there any plans to resolve this PR? It is now starting to be problem with Tailwind 3.3+ since their dark mode class strategy now uses |
|
I propose to take this over with #49169 |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
currently in emulated view encapsulation if a selector contains a
functional pseudo-class selector (such as :is() for example) and
contains different spaces and commas inside the parenthesis such
characters are handle as if there where part of a normal selector
thus causing various parsing issues, solve this issue by
escaping/unescaping the content of such selectors
Basically if a selector is:
:is(.foo, .bar):not(.baz)escape it to
:is_PLACEHOLDER_:not__PLACEHOLDER__,handle/scope it as normal and then
revert back the placeholders to the original values
resolves #45686
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Issue
Issue Number: #45686
Does this PR introduce a breaking change?