From 0ab78035fadaa7188b3c741f5f3c1a5440824f7f Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sat, 17 Aug 2019 10:22:55 +0200 Subject: [PATCH 01/10] Multi class with comma in a single expression --- src/compiler/compile/render_ssr/handlers/Element.ts | 4 ++-- src/runtime/internal/dom.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/compiler/compile/render_ssr/handlers/Element.ts b/src/compiler/compile/render_ssr/handlers/Element.ts index 146324f2a40a..97c8006fc891 100644 --- a/src/compiler/compile/render_ssr/handlers/Element.ts +++ b/src/compiler/compile/render_ssr/handlers/Element.ts @@ -86,8 +86,8 @@ export default function(node: Element, renderer: Renderer, options: RenderOption const class_expression = node.classes.map((class_directive: Class) => { const { expression, name } = class_directive; const snippet = expression ? snip(expression) : `ctx${quote_prop_if_necessary(name)}`; - return `${snippet} ? "${name}" : ""`; - }).join(', '); + return `(${snippet} ? "${name.replace(/,/g, ' ')} " : "")`; + }).join(' + ').trim(); let add_class_attribute = class_expression ? true : false; diff --git a/src/runtime/internal/dom.ts b/src/runtime/internal/dom.ts index 25c804ff34b4..51bbb74f3b71 100644 --- a/src/runtime/internal/dom.ts +++ b/src/runtime/internal/dom.ts @@ -238,7 +238,9 @@ export function add_resize_listener(element, fn) { } export function toggle_class(element, name, toggle) { - element.classList[toggle ? 'add' : 'remove'](name); + name.split(',').forEach(className => { + element.classList[toggle ? 'add' : 'remove'](className); + }) } export function custom_event(type: string, detail?: T) { From 55593546c190e767036bb50412354bdf15a77985 Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sat, 17 Aug 2019 10:23:44 +0200 Subject: [PATCH 02/10] Adding the new multi class support to docs --- site/content/docs/02-template-syntax.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/site/content/docs/02-template-syntax.md b/site/content/docs/02-template-syntax.md index 600634ab11d8..1aa3bed6e419 100644 --- a/site/content/docs/02-template-syntax.md +++ b/site/content/docs/02-template-syntax.md @@ -686,6 +686,13 @@ A `class:` directive provides a shorter way of toggling a class on an element.
...
``` +It accepts multiple classes in a single expression, separated by comma `,`. +However, the shorthand syntax cannot be used on this case. + +```html + +
...
+``` #### use:*action* From a065a23437f4635fcd8bf1e75b6e915edeb13446 Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sat, 17 Aug 2019 10:23:57 +0200 Subject: [PATCH 03/10] Creating some tests to check multi class feature --- .../class-with-multi-attribute/_config.js | 3 +++ .../class-with-multi-attribute/main.svelte | 1 + .../_config.js | 21 +++++++++++++++++++ .../main.svelte | 6 ++++++ .../_config.js | 15 +++++++++++++ .../main.svelte | 5 +++++ 6 files changed, 51 insertions(+) create mode 100644 test/runtime/samples/class-with-multi-attribute/_config.js create mode 100644 test/runtime/samples/class-with-multi-attribute/main.svelte create mode 100644 test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/_config.js create mode 100644 test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/main.svelte create mode 100644 test/runtime/samples/class-with-multi-dynamic-attribute/_config.js create mode 100644 test/runtime/samples/class-with-multi-dynamic-attribute/main.svelte diff --git a/test/runtime/samples/class-with-multi-attribute/_config.js b/test/runtime/samples/class-with-multi-attribute/_config.js new file mode 100644 index 000000000000..8657d48bde2e --- /dev/null +++ b/test/runtime/samples/class-with-multi-attribute/_config.js @@ -0,0 +1,3 @@ +export default { + html: `
` +}; diff --git a/test/runtime/samples/class-with-multi-attribute/main.svelte b/test/runtime/samples/class-with-multi-attribute/main.svelte new file mode 100644 index 000000000000..6ddc57f62a35 --- /dev/null +++ b/test/runtime/samples/class-with-multi-attribute/main.svelte @@ -0,0 +1 @@ +
\ No newline at end of file diff --git a/test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/_config.js b/test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/_config.js new file mode 100644 index 000000000000..a90c6875cc36 --- /dev/null +++ b/test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/_config.js @@ -0,0 +1,21 @@ +export default { + props: { + myClass: 'one two', + attributes: { + role: 'button' + } + }, + + html: `
`, + + test({ assert, component, target, window }) { + component.myClass = 'one'; + component.attributes = { + 'aria-label': 'Test' + }; + + assert.htmlEqual(target.innerHTML, ` +
+ `); + } +}; diff --git a/test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/main.svelte b/test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/main.svelte new file mode 100644 index 000000000000..50836d842035 --- /dev/null +++ b/test/runtime/samples/class-with-multi-dynamic-attribute-and-spread/main.svelte @@ -0,0 +1,6 @@ + + +
diff --git a/test/runtime/samples/class-with-multi-dynamic-attribute/_config.js b/test/runtime/samples/class-with-multi-dynamic-attribute/_config.js new file mode 100644 index 000000000000..a2dbb5ed538d --- /dev/null +++ b/test/runtime/samples/class-with-multi-dynamic-attribute/_config.js @@ -0,0 +1,15 @@ +export default { + props: { + myClass: 'one two' + }, + + html: `
`, + + test({ assert, component, target, window }) { + component.myClass = 'one'; + + assert.htmlEqual(target.innerHTML, ` +
+ `); + } +}; diff --git a/test/runtime/samples/class-with-multi-dynamic-attribute/main.svelte b/test/runtime/samples/class-with-multi-dynamic-attribute/main.svelte new file mode 100644 index 000000000000..5a5403cfaf76 --- /dev/null +++ b/test/runtime/samples/class-with-multi-dynamic-attribute/main.svelte @@ -0,0 +1,5 @@ + + +
\ No newline at end of file From 9804f58b5af2029341a0718953fae630dfad34f4 Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sat, 17 Aug 2019 11:27:00 +0200 Subject: [PATCH 04/10] Removing injection of multiple class attrs --- src/compiler/compile/render_ssr/handlers/Element.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/compile/render_ssr/handlers/Element.ts b/src/compiler/compile/render_ssr/handlers/Element.ts index 97c8006fc891..fafe5a3fb2df 100644 --- a/src/compiler/compile/render_ssr/handlers/Element.ts +++ b/src/compiler/compile/render_ssr/handlers/Element.ts @@ -110,6 +110,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption // a boolean attribute with one non-Text chunk args.push(`{ ${quote_name_if_necessary(attribute.name)}: ${snip(attribute.chunks[0])} }`); } else if (attribute.name === 'class' && class_expression) { + add_class_attribute = false; // Add class expression args.push(`{ ${quote_name_if_necessary(attribute.name)}: [\`${stringify_class_attribute(attribute)}\`, \`\${${class_expression}}\`].join(' ').trim() }`); } else { From 58053cff320a5cc9b4da0aba2637efa5da6ece6f Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sun, 25 Aug 2019 00:48:48 +0200 Subject: [PATCH 05/10] Warning if a class name is used multiple times --- src/compiler/compile/nodes/Element.ts | 43 +++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src/compiler/compile/nodes/Element.ts b/src/compiler/compile/nodes/Element.ts index 593f303a2fc5..a8bc6e539a87 100644 --- a/src/compiler/compile/nodes/Element.ts +++ b/src/compiler/compile/nodes/Element.ts @@ -181,12 +181,12 @@ export default class Element extends Node { break; case 'Transition': - { - const transition = new Transition(component, this, scope, node); - if (node.intro) this.intro = transition; - if (node.outro) this.outro = transition; - break; - } + { + const transition = new Transition(component, this, scope, node); + if (node.intro) this.intro = transition; + if (node.outro) this.outro = transition; + break; + } case 'Animation': this.animation = new Animation(component, this, scope, node); @@ -268,6 +268,7 @@ export default class Element extends Node { } this.validate_attributes(); + this.validate_classes(); this.validate_bindings(); this.validate_content(); this.validate_event_handlers(); @@ -470,6 +471,36 @@ export default class Element extends Node { } } + validate_classes() { + const classAttribute = this.attributes.find( + attribute => !attribute.is_spread && attribute.name.toLowerCase() === "class" + ); + + let value: string | true = ''; + + if (classAttribute) { + value = classAttribute.get_static_value() + } + + const classNames = String(value).split(" "); + + this.classes.forEach(class_directive => { + const { name } = class_directive; + + name.split(",").forEach(className => { + if (classNames.includes(className)) { + this.component.warn(this, { + code: `class-name-multiple-attrs`, + message: `Class: avoid using class name '${className}' in more than one class attribute` + }); + + } else { + classNames.push(className); + } + }) + }) + } + validate_bindings() { const { component } = this; From b085662b8e02215dfbdb6587a01ba102470ab952 Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sun, 25 Aug 2019 00:49:16 +0200 Subject: [PATCH 06/10] Adding tests to check if has multiple class with same name --- .../input.svelte | 3 +++ .../warnings.json | 17 +++++++++++++++++ .../class-name-multiple-attrs/input.svelte | 5 +++++ .../class-name-multiple-attrs/warnings.json | 17 +++++++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 test/validator/samples/class-multi-name-multiple-attrs/input.svelte create mode 100644 test/validator/samples/class-multi-name-multiple-attrs/warnings.json create mode 100644 test/validator/samples/class-name-multiple-attrs/input.svelte create mode 100644 test/validator/samples/class-name-multiple-attrs/warnings.json diff --git a/test/validator/samples/class-multi-name-multiple-attrs/input.svelte b/test/validator/samples/class-multi-name-multiple-attrs/input.svelte new file mode 100644 index 000000000000..12c7723bc34b --- /dev/null +++ b/test/validator/samples/class-multi-name-multiple-attrs/input.svelte @@ -0,0 +1,3 @@ +
+ Hello World +
diff --git a/test/validator/samples/class-multi-name-multiple-attrs/warnings.json b/test/validator/samples/class-multi-name-multiple-attrs/warnings.json new file mode 100644 index 000000000000..9310fd0626ac --- /dev/null +++ b/test/validator/samples/class-multi-name-multiple-attrs/warnings.json @@ -0,0 +1,17 @@ +[ + { + "code": "class-name-multiple-attrs", + "message": "Class: avoid using class name 'two' in more than one class attribute", + "start": { + "character": 0, + "column": 0, + "line": 1 + }, + "end": { + "character": 74, + "column": 6, + "line": 3 + }, + "pos": 0 + } +] diff --git a/test/validator/samples/class-name-multiple-attrs/input.svelte b/test/validator/samples/class-name-multiple-attrs/input.svelte new file mode 100644 index 000000000000..c46b5e030a47 --- /dev/null +++ b/test/validator/samples/class-name-multiple-attrs/input.svelte @@ -0,0 +1,5 @@ + + +
diff --git a/test/validator/samples/class-name-multiple-attrs/warnings.json b/test/validator/samples/class-name-multiple-attrs/warnings.json new file mode 100644 index 000000000000..d1f88f85258f --- /dev/null +++ b/test/validator/samples/class-name-multiple-attrs/warnings.json @@ -0,0 +1,17 @@ +[ + { + "code": "class-name-multiple-attrs", + "message": "Class: avoid using class name 'one' in more than one class attribute", + "start": { + "character": 30, + "column": 0, + "line": 5 + }, + "end": { + "character": 58, + "column": 28, + "line": 5 + }, + "pos": 30 + } +] From 3c212252bb899904b07dcb8746fae5add5e280dd Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sun, 25 Aug 2019 00:52:30 +0200 Subject: [PATCH 07/10] Removing lint without affecting the feature --- src/compiler/compile/nodes/Element.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/compiler/compile/nodes/Element.ts b/src/compiler/compile/nodes/Element.ts index a8bc6e539a87..a7571f75d182 100644 --- a/src/compiler/compile/nodes/Element.ts +++ b/src/compiler/compile/nodes/Element.ts @@ -181,12 +181,12 @@ export default class Element extends Node { break; case 'Transition': - { - const transition = new Transition(component, this, scope, node); - if (node.intro) this.intro = transition; - if (node.outro) this.outro = transition; - break; - } + { + const transition = new Transition(component, this, scope, node); + if (node.intro) this.intro = transition; + if (node.outro) this.outro = transition; + break; + } case 'Animation': this.animation = new Animation(component, this, scope, node); From 5df2032abecc1b40a22add37f1e99822c66c108c Mon Sep 17 00:00:00 2001 From: Marcelo Junior Date: Sun, 25 Aug 2019 01:04:56 +0200 Subject: [PATCH 08/10] Adding explanation on docs why avoid same name in multi class attrs --- site/content/docs/02-template-syntax.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/site/content/docs/02-template-syntax.md b/site/content/docs/02-template-syntax.md index 1aa3bed6e419..a8007ec48e42 100644 --- a/site/content/docs/02-template-syntax.md +++ b/site/content/docs/02-template-syntax.md @@ -694,6 +694,9 @@ However, the shorthand syntax cannot be used on this case.
...
``` +> You should take care when using multiple `class` attributes, because it can affect each other. Like when using `class="foo" class:foo`, the second expression can remove the first one. Svelte warns when it finds the same name in multiple `class` expressions. + + #### use:*action* ```sv From d4c63df13190081202c8fd6877e55f00368729e7 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Thu, 14 Nov 2019 22:33:24 -0500 Subject: [PATCH 09/10] get most tests passing --- .../compile/render_ssr/handlers/Element.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/compiler/compile/render_ssr/handlers/Element.ts b/src/compiler/compile/render_ssr/handlers/Element.ts index f79a69af6696..12dc87959f6e 100644 --- a/src/compiler/compile/render_ssr/handlers/Element.ts +++ b/src/compiler/compile/render_ssr/handlers/Element.ts @@ -31,7 +31,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption const class_expression_list = node.classes.map(class_directive => { const { expression, name } = class_directive; const snippet = expression ? expression.node : x`#ctx.${name}`; - return x`${snippet} ? "${name}" : ""`; + return x`${snippet} ? "${name}".replace(/,/g, ' ') : ""`; }); if (node.needs_manual_style_scoping) { class_expression_list.push(x`"${node.component.stylesheet.id}"`); @@ -41,6 +41,8 @@ export default function(node: Element, renderer: Renderer, options: RenderOption class_expression_list.length > 0 && class_expression_list.reduce((lhs, rhs) => x`${lhs} + ' ' + ${rhs}`); + let add_class_attribute = !!class_expression; + if (node.attributes.some(attr => attr.is_spread)) { // TODO dry this out const args = []; @@ -62,6 +64,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption args.push(x`{ ${attribute.name}: ${(attribute.chunks[0] as Expression).node} || null }`); } else if (attribute.name === 'class' && class_expression) { // Add class expression + add_class_attribute = false; args.push(x`{ ${attribute.name}: [${get_attribute_value(attribute)}, ${class_expression}].join(' ').trim() }`); } else { args.push(x`{ ${attribute.name}: ${get_attribute_value(attribute)} }`); @@ -69,9 +72,8 @@ export default function(node: Element, renderer: Renderer, options: RenderOption } }); - renderer.add_expression(x`@spread([${args}], ${class_expression});`); + renderer.add_expression(x`@spread([${args}], ${add_class_attribute && class_expression});`); } else { - let add_class_attribute = !!class_expression; node.attributes.forEach(attribute => { const name = attribute.name.toLowerCase(); if (name === 'value' && node.name.toLowerCase() === 'textarea') { @@ -100,9 +102,10 @@ export default function(node: Element, renderer: Renderer, options: RenderOption renderer.add_string(`"`); } }); - if (add_class_attribute) { - renderer.add_expression(x`@add_classes([${class_expression}].join(' ').trim())`); - } + } + + if (add_class_attribute) { + renderer.add_expression(x`@add_classes([${class_expression}].join(' ').trim())`); } node.bindings.forEach(binding => { From 9d424fea693e1d254147056a688f438886e63a8a Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Thu, 14 Nov 2019 22:34:05 -0500 Subject: [PATCH 10/10] snake_case --- src/compiler/compile/nodes/Element.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/compiler/compile/nodes/Element.ts b/src/compiler/compile/nodes/Element.ts index cf60c5537530..50b2589c3ab2 100644 --- a/src/compiler/compile/nodes/Element.ts +++ b/src/compiler/compile/nodes/Element.ts @@ -477,30 +477,30 @@ export default class Element extends Node { } validate_classes() { - const classAttribute = this.attributes.find( + const class_attribute = this.attributes.find( attribute => !attribute.is_spread && attribute.name.toLowerCase() === "class" ); let value: string | true = ''; - if (classAttribute) { - value = classAttribute.get_static_value() + if (class_attribute) { + value = class_attribute.get_static_value() } - const classNames = String(value).split(" "); + const class_names = String(value).split(" "); this.classes.forEach(class_directive => { const { name } = class_directive; - name.split(",").forEach(className => { - if (classNames.includes(className)) { + name.split(",").forEach(class_name => { + if (class_names.includes(class_name)) { this.component.warn(this, { code: `class-name-multiple-attrs`, - message: `Class: avoid using class name '${className}' in more than one class attribute` + message: `Class: avoid using class name '${class_name}' in more than one class attribute` }); } else { - classNames.push(className); + class_names.push(class_name); } }) })