Skip to content
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

Bug in adding a new css rule with selectors #23

Closed
fgiannar opened this issue Oct 14, 2016 · 5 comments
Closed

Bug in adding a new css rule with selectors #23

fgiannar opened this issue Oct 14, 2016 · 5 comments
Labels

Comments

@fgiannar
Copy link

In css_composer/main.js line 173 (add):

rule.get('selectors').add(selectors);

returns selectors with empty ids..

instead

selectors.each(function(selector){ rule.get('selectors').add(selector); });

seems to work.

@artf
Copy link
Member

artf commented Oct 14, 2016

Hi @fgiannar
sorry but I can't reproduce the bug
Anyway, the return of rule.get('selectors').add(selectors) should be the "same" selectors array

var sm = editor.SelectorManager;
var sel1 = sm.add('myClass1');
var sel2 = sm.add('myClass2');
var selectors = [sel1, sel2]; 

var cssComposer = editor.CssComposer;
var rule = cssComposer.add(selectors, 'hover');
//...
// And inside the add()... when you are at 173 line:
var addedSelectors = rule.get('selectors').add(selectors); 
/*
'addedSelectors' and 'selectors' are 2 different arrays but contain
the same instances of sel1 and sel2
*/

Do you have some use case inside the editor where I can catch some strange behaviour?

@fgiannar
Copy link
Author

No I'm afraid not. This occurred from a custom script of mine, where I had:
cssComposer.add(target.get('selectors'), '', '480px'); where target is an instance of a CssRule with maxWidth not set.

If I replaced the line 173, as explained in my first comment above, the selector models had ids else they had the name attribute set but empty id which lead me to think it's a general bug.

Probably my bad, I am closing it since it's not reproducible on your end.
Thanks a lot for your prompt response.

@artf
Copy link
Member

artf commented Oct 17, 2016

Hi @fgiannar I think I get it, target.get('selectors') returns a Backbone Collection but cssComposer.add accepts Array as the first argument, so you should fix with this:
cssComposer.add(target.get('selectors').models, '', '480px');

@fgiannar
Copy link
Author

Just an update, in case anyone else faces the same issue:
The mistake was passing the selectors collection instead of array of models, so replacing:
cssComposer.add(target.get('selectors'), '', '480px');
with
cssComposer.add(target.get('selectors').models, '', '480px');
fixed the issue.

Again thanks for your time.

@lock
Copy link

lock bot commented Sep 18, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the outdated label Sep 18, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants