-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
maping through nested rules #2935
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please explain the point of this PR? What are you trying to solve?
@@ -107,11 +107,15 @@ export default Backbone.Model.extend({ | |||
const selectorStrNoAdd = rule.selectorsToString({ skipAdd: 1 }); | |||
const selectorsAdd = rule.get('selectorsAdd'); | |||
const singleAtRule = rule.get('singleAtRule'); | |||
let isGjsRule = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the point of this variable? Seems like you don't use it...
@@ -15,7 +15,7 @@ const atRules = { | |||
}; | |||
const atRuleKeys = keys(atRules); | |||
const singleAtRules = ['5', '6', '11', '15']; | |||
const singleAtRulesNames = ['font-face', 'page', 'counter-style', 'viewport']; | |||
const singleAtRulesNames = ['font-face', 'counter-style', 'viewport']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove the page
?
@@ -121,8 +125,10 @@ export default Backbone.Model.extend({ | |||
} | |||
}); | |||
|
|||
if ((selectorStrNoAdd && found) || selectorsAdd || singleAtRule) { | |||
const block = rule.getDeclaration(); | |||
if ((selectorStrNoAdd && found) || selectorsAdd || singleAtRule || rule.attributes.style) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rule.attributes.style
is an object, it's a truthy value, so the condition is pointless
if ((selectorStrNoAdd && found) || selectorsAdd || singleAtRule || rule.attributes.style) { | ||
const entries = Object.entries(rule.attributes.style); | ||
const styleStr = entries.length > 0 ? entries.map(([k, v]) => `${k}:${v}`).join(';') + ';' : ''; | ||
const block = rule.getDeclaration().length > 0 ? rule.getDeclaration() : styleStr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're calling rule.getDeclaration() twice and honestly, I don't understand the point of this change
Thanks for the review. I will make some correction and more updates. |
This is integration of the nested rules. When nested at rules is parsed from grapesjs-parset-postcss, grapes js resolve them.
an example is also provided in the postcss repo.