Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Commit 607b35d

Browse files
author
Narciso Jaramillo
committed
Only show one entry in rule list for rules with multiple matching selectors
1 parent 340ae12 commit 607b35d

3 files changed

Lines changed: 145 additions & 4 deletions

File tree

‎src/editor/CSSInlineEditor.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,10 @@ define(function (require, exports, module) {
243243
});
244244
return result;
245245
}
246-
246+
247247
CSSUtils.findMatchingRules(selectorName, hostEditor.document)
248248
.done(function (rules) {
249-
cssInlineEditor = new MultiRangeInlineEditor(rules || [], _getNoRulesMsg);
249+
cssInlineEditor = new MultiRangeInlineEditor(CSSUtils.consolidateRules(rules) || [], _getNoRulesMsg);
250250
cssInlineEditor.load(hostEditor);
251251

252252
var $header = $(".inline-editor-header", cssInlineEditor.$htmlContent);

‎src/language/CSSUtils.js‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,8 @@ define(function (require, exports, module) {
504504
starts that this selector (e.g. .baz) is part of. Particularly relevant for
505505
groups that are on multiple lines.
506506
selectorGroupStartChar: column in line where the selector group starts.
507+
selectorGroup: the entire selector group containing this selector, or undefined if there
508+
is only one selector in the rule.
507509
declListStartLine: line where the declaration list for the rule starts
508510
declListStartChar: column in line where the declaration list for the rule starts
509511
declListEndLine: line where the declaration list for the rule ends
@@ -690,6 +692,25 @@ define(function (require, exports, module) {
690692
var j;
691693
declListStartLine = line;
692694
declListStartChar = stream.start;
695+
696+
// Extract the entire selector group we just saw.
697+
var selectorGroup, sgLine;
698+
if (selectorGroupStartLine !== -1) {
699+
selectorGroup = "";
700+
for (sgLine = selectorGroupStartLine; sgLine <= declListStartLine; sgLine++) {
701+
var startChar = 0, endChar = lines[sgLine].length;
702+
if (sgLine === selectorGroupStartLine) {
703+
startChar = selectorGroupStartChar;
704+
} else {
705+
selectorGroup += " "; // replace the newline with a single space
706+
}
707+
if (sgLine === declListStartLine) {
708+
endChar = declListStartChar;
709+
}
710+
selectorGroup += lines[sgLine].substring(startChar, endChar);
711+
}
712+
selectorGroup = selectorGroup.trim();
713+
}
693714

694715
// Since we're now in a declaration list, that means we also finished
695716
// parsing the whole selector group. Therefore, reset selectorGroupStartLine
@@ -706,7 +727,7 @@ define(function (require, exports, module) {
706727
}
707728
}
708729

709-
// assign this declaration list position to every selector on the stack
730+
// assign this declaration list position and selector group to every selector on the stack
710731
// that doesn't have a declaration list start and end line
711732
for (j = selectors.length - 1; j >= 0; j--) {
712733
if (selectors[j].declListEndLine !== -1) {
@@ -716,6 +737,9 @@ define(function (require, exports, module) {
716737
selectors[j].declListStartChar = declListStartChar;
717738
selectors[j].declListEndLine = line;
718739
selectors[j].declListEndChar = stream.pos - 1; // stream.pos actually points to the char after the }
740+
if (selectorGroup) {
741+
selectors[j].selectorGroup = selectorGroup;
742+
}
719743
}
720744
}
721745
}
@@ -915,7 +939,8 @@ define(function (require, exports, module) {
915939
name: selectorInfo.selector,
916940
document: sourceDoc,
917941
lineStart: selectorInfo.ruleStartLine + lineOffset,
918-
lineEnd: selectorInfo.declListEndLine + lineOffset
942+
lineEnd: selectorInfo.declListEndLine + lineOffset,
943+
selectorGroup: selectorInfo.selectorGroup
919944
});
920945
});
921946
}
@@ -1220,13 +1245,37 @@ define(function (require, exports, module) {
12201245
};
12211246
}
12221247

1248+
/**
1249+
*
1250+
* In the given rule array (as returned by `findMatchingRules()`), if multiple rules in a row
1251+
* refer to the same rule (because there were multiple matching selectors), eliminate the redundant
1252+
* rules and use the selectorGroup as the name instead of the original selector.
1253+
*/
1254+
function consolidateRules(rules) {
1255+
var newRules = [], lastRule;
1256+
rules.forEach(function (rule) {
1257+
if (lastRule &&
1258+
rule.document === lastRule.document &&
1259+
rule.lineStart === lastRule.lineStart &&
1260+
rule.lineEnd === lastRule.lineEnd &&
1261+
rule.selectorGroup === lastRule.selectorGroup) {
1262+
lastRule.name = lastRule.selectorGroup;
1263+
} else {
1264+
newRules.push(rule);
1265+
}
1266+
lastRule = rule;
1267+
});
1268+
return newRules;
1269+
}
1270+
12231271
exports._findAllMatchingSelectorsInText = _findAllMatchingSelectorsInText; // For testing only
12241272
exports.findMatchingRules = findMatchingRules;
12251273
exports.extractAllSelectors = extractAllSelectors;
12261274
exports.extractAllNamedFlows = extractAllNamedFlows;
12271275
exports.findSelectorAtDocumentPos = findSelectorAtDocumentPos;
12281276
exports.reduceStyleSheetForRegExParsing = reduceStyleSheetForRegExParsing;
12291277
exports.addRuleToDocument = addRuleToDocument;
1278+
exports.consolidateRules = consolidateRules;
12301279

12311280
exports.SELECTOR = SELECTOR;
12321281
exports.PROP_NAME = PROP_NAME;

‎test/spec/CSSUtils-test.js‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,7 @@ define(function (require, exports, module) {
646646
it("should match a lone type selector given a type", function () {
647647
var result = match("div { color:red }", { tag: "div" });
648648
expect(result.length).toBe(1);
649+
expect(result[0].selectorGroup).toBeUndefined();
649650

650651
result = matchAgain({ tag: "span" });
651652
expect(result.length).toBe(0);
@@ -660,6 +661,7 @@ define(function (require, exports, module) {
660661
it("should match a lone class selector given a class", function () {
661662
var result = match(".foo { color:red }", { clazz: "foo" });
662663
expect(result.length).toBe(1);
664+
expect(result[0].selectorGroup).toBeUndefined();
663665

664666
result = matchAgain({ clazz: "bar" });
665667
expect(result.length).toBe(0);
@@ -677,6 +679,7 @@ define(function (require, exports, module) {
677679
it("should match a lone id selector given an id", function () {
678680
var result = match("#foo { color:red }", { id: "foo" });
679681
expect(result.length).toBe(1);
682+
expect(result[0].selectorGroup).toBeUndefined();
680683

681684
result = matchAgain({ id: "bar" });
682685
expect(result.length).toBe(0);
@@ -698,6 +701,7 @@ define(function (require, exports, module) {
698701

699702
var result = match(css, { tag: "div" });
700703
expect(result.length).toBe(1);
704+
expect(result[0].selectorGroup).toBeUndefined();
701705
result = matchAgain({ tag: "foo" });
702706
expect(result.length).toBe(0);
703707
result = matchAgain({ tag: "bar" });
@@ -707,6 +711,7 @@ define(function (require, exports, module) {
707711
expect(result.length).toBe(0);
708712
result = matchAgain({ clazz: "foo" });
709713
expect(result.length).toBe(1);
714+
expect(result[0].selectorGroup).toBeUndefined();
710715
result = matchAgain({ clazz: "bar" });
711716
expect(result.length).toBe(0);
712717

@@ -716,6 +721,7 @@ define(function (require, exports, module) {
716721
expect(result.length).toBe(0);
717722
result = matchAgain({ id: "bar" });
718723
expect(result.length).toBe(1);
724+
expect(result[0].selectorGroup).toBeUndefined();
719725
});
720726

721727
it("should be case-sensitive for all but types", function () {
@@ -1173,13 +1179,15 @@ define(function (require, exports, module) {
11731179
expect(result.length).toBe(0);
11741180
result = matchAgain({ clazz: "foo" });
11751181
expect(result.length).toBe(1);
1182+
expect(result[0].selectorGroup).toBeUndefined();
11761183

11771184
result = match("p h4 div { color:red }", { tag: "p" });
11781185
expect(result.length).toBe(0);
11791186
result = matchAgain({ tag: "h4" });
11801187
expect(result.length).toBe(0);
11811188
result = matchAgain({ tag: "div" });
11821189
expect(result.length).toBe(1);
1190+
expect(result[0].selectorGroup).toBeUndefined();
11831191

11841192
result = match(".foo h4 { color:red }", { tag: "h4" });
11851193
expect(result.length).toBe(1);
@@ -1188,8 +1196,10 @@ define(function (require, exports, module) {
11881196

11891197
result = match("div div { color:red }", { tag: "div" });
11901198
expect(result.length).toBe(1);
1199+
expect(result[0].selectorGroup).toBeUndefined();
11911200
result = match(".foo .foo { color:red }", { clazz: "foo" });
11921201
expect(result.length).toBe(1);
1202+
expect(result[0].selectorGroup).toBeUndefined();
11931203
result = matchAgain({ tag: "foo" });
11941204
expect(result.length).toBe(0);
11951205
});
@@ -1199,6 +1209,7 @@ define(function (require, exports, module) {
11991209
expect(result.length).toBe(0);
12001210
result = matchAgain({ clazz: "foo" });
12011211
expect(result.length).toBe(1);
1212+
expect(result[0].selectorGroup).toBeUndefined();
12021213

12031214
result = match(".foo > h4 { color:red }", { tag: "h4" });
12041215
expect(result.length).toBe(1);
@@ -1280,42 +1291,63 @@ define(function (require, exports, module) {
12801291
// Comma- and space- separated
12811292
var result = match("h4, .foo, #bar { color:red }", { tag: "h4" });
12821293
expect(result.length).toBe(1);
1294+
expect(result[0].selectorGroup).toBe("h4, .foo, #bar");
12831295
result = matchAgain({ clazz: "foo" });
12841296
expect(result.length).toBe(1);
1297+
expect(result[0].selectorGroup).toBe("h4, .foo, #bar");
12851298
result = matchAgain({ id: "bar" });
12861299
expect(result.length).toBe(1);
1300+
expect(result[0].selectorGroup).toBe("h4, .foo, #bar");
12871301

12881302
// Comma only
12891303
result = match("h4,.foo,#bar { color:red }", { tag: "h4" });
12901304
expect(result.length).toBe(1);
1305+
expect(result[0].selectorGroup).toBe("h4,.foo,#bar");
12911306
result = matchAgain({ clazz: "foo" });
12921307
expect(result.length).toBe(1);
1308+
expect(result[0].selectorGroup).toBe("h4,.foo,#bar");
12931309
result = matchAgain({ id: "bar" });
12941310
expect(result.length).toBe(1);
1311+
expect(result[0].selectorGroup).toBe("h4,.foo,#bar");
12951312

12961313
// Newline-separated
12971314
result = match("h4,\n.foo,\r\n#bar { color:red }", { tag: "h4" });
12981315
expect(result.length).toBe(1);
1316+
expect(result[0].selectorGroup).toBe("h4, .foo, #bar");
12991317
result = matchAgain({ clazz: "foo" });
13001318
expect(result.length).toBe(1);
1319+
expect(result[0].selectorGroup).toBe("h4, .foo, #bar");
13011320
result = matchAgain({ id: "bar" });
13021321
expect(result.length).toBe(1);
1322+
expect(result[0].selectorGroup).toBe("h4, .foo, #bar");
13031323

13041324
// Space-separated with a space combinator
13051325
result = match("h4, .foo #bar { color:red }", { tag: "h4" });
13061326
expect(result.length).toBe(1);
1327+
expect(result[0].selectorGroup).toBe("h4, .foo #bar");
13071328
result = matchAgain({ clazz: "foo" });
13081329
expect(result.length).toBe(0);
13091330
result = matchAgain({ id: "bar" });
13101331
expect(result.length).toBe(1);
1332+
expect(result[0].selectorGroup).toBe("h4, .foo #bar");
13111333

13121334
// Test items of each type in all positions (first, last, middle)
13131335
result = match("h4, h4, h4 { color:red }", { tag: "h4" });
13141336
expect(result.length).toBe(3);
1337+
var i;
1338+
for (i = 0; i < 3; i++) {
1339+
expect(result[i].selectorGroup).toBe("h4, h4, h4");
1340+
}
13151341
result = match(".foo, .foo, .foo { color:red }", { clazz: "foo" });
13161342
expect(result.length).toBe(3);
1343+
for (i = 0; i < 3; i++) {
1344+
expect(result[i].selectorGroup).toBe(".foo, .foo, .foo");
1345+
}
13171346
result = match("#bar, #bar, #bar { color:red }", { id: "bar" });
13181347
expect(result.length).toBe(3);
1348+
for (i = 0; i < 3; i++) {
1349+
expect(result[i].selectorGroup).toBe("#bar, #bar, #bar");
1350+
}
13191351
});
13201352
}); // describe("Selector groups")
13211353

@@ -1561,6 +1593,66 @@ define(function (require, exports, module) {
15611593
}
15621594
});
15631595
});
1596+
1597+
it("should consolidate consecutive rules that refer to the same item", function () {
1598+
var doc1 = SpecRunnerUtils.createMockDocument(""),
1599+
doc2 = SpecRunnerUtils.createMockDocument(""),
1600+
rules = [
1601+
{
1602+
name: ".foo",
1603+
doc: doc1,
1604+
lineStart: 5,
1605+
lineEnd: 7
1606+
},
1607+
{
1608+
name: ".eek",
1609+
doc: doc1,
1610+
lineStart: 10,
1611+
lineEnd: 12
1612+
},
1613+
{
1614+
name: ".bar",
1615+
doc: doc2,
1616+
lineStart: 3,
1617+
lineEnd: 5
1618+
},
1619+
{
1620+
name: "#baz",
1621+
doc: doc2,
1622+
lineStart: 8,
1623+
lineEnd: 12,
1624+
selectorGroup: "#baz, h2"
1625+
},
1626+
{
1627+
name: "h2",
1628+
doc: doc2,
1629+
lineStart: 8,
1630+
lineEnd: 12,
1631+
selectorGroup: "#baz, h2"
1632+
},
1633+
{
1634+
name: ".argle",
1635+
doc: doc2,
1636+
lineStart: 15,
1637+
lineEnd: 20
1638+
}
1639+
],
1640+
result = CSSUtils.consolidateRules(rules);
1641+
1642+
expect(result).toEqual([
1643+
rules[0],
1644+
rules[1],
1645+
rules[2],
1646+
{
1647+
name: "#baz, h2",
1648+
doc: doc2,
1649+
lineStart: 8,
1650+
lineEnd: 12,
1651+
selectorGroup: "#baz, h2"
1652+
},
1653+
rules[5]
1654+
]);
1655+
});
15641656
});
15651657

15661658
// Unit Tests: "HTMLUtils (css)"

0 commit comments

Comments
 (0)
⚡