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

Multiple classes with class:x,y,z={condition} #3419

Closed
wants to merge 12 commits into from
Closed
10 changes: 10 additions & 0 deletions site/content/docs/02-template-syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,16 @@ A `class:` directive provides a shorter way of toggling a class on an element.
<div class:active class:inactive={!active} class:isAdmin>...</div>
```

It accepts multiple classes in a single expression, separated by comma `,`.
However, the shorthand syntax cannot be used on this case.

```html
<!-- Toggle both active and primary classes -->
<div class:active,primary="{isActive}">...</div>
```

> 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*

Expand Down
31 changes: 31 additions & 0 deletions src/compiler/compile/nodes/Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ export default class Element extends Node {
}

this.validate_attributes();
this.validate_classes();
this.validate_bindings();
this.validate_content();
this.validate_event_handlers();
Expand Down Expand Up @@ -475,6 +476,36 @@ export default class Element extends Node {
}
}

validate_classes() {
const class_attribute = this.attributes.find(
attribute => !attribute.is_spread && attribute.name.toLowerCase() === "class"
);

let value: string | true = '';

if (class_attribute) {
value = class_attribute.get_static_value()
}

const class_names = String(value).split(" ");

this.classes.forEach(class_directive => {
const { name } = class_directive;

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 '${class_name}' in more than one class attribute`
});

} else {
class_names.push(class_name);
}
})
})
}

validate_bindings() {
const { component } = this;

Expand Down
19 changes: 13 additions & 6 deletions src/compiler/compile/render_ssr/handlers/Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ 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}"`);
}

const class_expression =
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 = [];
Expand All @@ -59,15 +62,18 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
) {
// a boolean attribute with one non-Text chunk
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)} }`);
}
}
});

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') {
Expand Down Expand Up @@ -96,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 => {
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/internal/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,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<T=any>(type: string, detail?: T) {
Expand Down
3 changes: 3 additions & 0 deletions test/runtime/samples/class-with-multi-attribute/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {
html: `<div class="one two three four"></div>`
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<div class="one" class:two={true} class:three,four={true}></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
export default {
props: {
myClass: 'one two',
attributes: {
role: 'button'
}
},

html: `<div class="one two three four" role="button"></div>`,

test({ assert, component, target, window }) {
component.myClass = 'one';
component.attributes = {
'aria-label': 'Test'
};

assert.htmlEqual(target.innerHTML, `
<div class="one three four" aria-label="Test"></div>
`);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
export let myClass;
export let attributes = {};
</script>

<div class={myClass} class:three,four={true} {...attributes}></div>
15 changes: 15 additions & 0 deletions test/runtime/samples/class-with-multi-dynamic-attribute/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export default {
props: {
myClass: 'one two'
},

html: `<div class="one two three four"></div>`,

test({ assert, component, target, window }) {
component.myClass = 'one';

assert.htmlEqual(target.innerHTML, `
<div class="one three four"></div>
`);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
export let myClass;
</script>

<div class={myClass} class:three,four={true}></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div class:one,two="{true}" class:two,three="{false}">
Hello World
</div>
Original file line number Diff line number Diff line change
@@ -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
}
]
5 changes: 5 additions & 0 deletions test/validator/samples/class-name-multiple-attrs/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let one;
</script>

<div class="one" class:one/>
17 changes: 17 additions & 0 deletions test/validator/samples/class-name-multiple-attrs/warnings.json
Original file line number Diff line number Diff line change
@@ -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
}
]