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

feat: warn on using slide transition with invalid display styles #14936

Merged
merged 3 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silver-humans-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: warn on using `slide` transition with table elements
12 changes: 12 additions & 0 deletions documentation/docs/98-reference/.generated/client-warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,15 @@ Reactive `$state(...)` proxies and the values they proxy have different identiti
```

To resolve this, ensure you're comparing values where both values were created with `$state(...)`, or neither were. Note that `$state.raw(...)` will _not_ create a state proxy.

### transition_slide_display

```
The `slide` transition does not work correctly for elements with `display: %value%`
```

The [slide](/docs/svelte/svelte-transition#slide) transition works by animating the `height` of the element, which requires a `display` style like `block`, `flex` or `grid`. It does not work for:

- `display: inline` (which is the default for elements like `<span>`), and its variants like `inline-block`, `inline-flex` and `inline-grid`
- `display: table` and `table-[name]`, which are the defaults for elements like `<table>` and `<tr>`
- `display: contents`
10 changes: 10 additions & 0 deletions packages/svelte/messages/client-warnings/warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,13 @@ To fix it, either create callback props to communicate changes, or mark `person`
```

To resolve this, ensure you're comparing values where both values were created with `$state(...)`, or neither were. Note that `$state.raw(...)` will _not_ create a state proxy.

## transition_slide_display

> The `slide` transition does not work correctly for elements with `display: %value%`

The [slide](/docs/svelte/svelte-transition#slide) transition works by animating the `height` of the element, which requires a `display` style like `block`, `flex` or `grid`. It does not work for:

- `display: inline` (which is the default for elements like `<span>`), and its variants like `inline-block`, `inline-flex` and `inline-grid`
- `display: table` and `table-[name]`, which are the defaults for elements like `<table>` and `<tr>`
- `display: contents`
12 changes: 12 additions & 0 deletions packages/svelte/src/internal/client/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,16 @@ export function state_proxy_equality_mismatch(operator) {
} else {
console.warn(`https://svelte.dev/e/state_proxy_equality_mismatch`);
}
}

/**
* The `slide` transition does not work correctly for elements with `display: %value%`
* @param {string} value
*/
export function transition_slide_display(value) {
if (DEV) {
console.warn(`%c[svelte] transition_slide_display\n%cThe \`slide\` transition does not work correctly for elements with \`display: ${value}\`\nhttps://svelte.dev/e/transition_slide_display`, bold, normal);
} else {
console.warn(`https://svelte.dev/e/transition_slide_display`);
}
}
13 changes: 13 additions & 0 deletions packages/svelte/src/transition/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
/** @import { BlurParams, CrossfadeParams, DrawParams, FadeParams, FlyParams, ScaleParams, SlideParams, TransitionConfig } from './public' */

import { DEV } from 'esm-env';
import * as w from '../internal/client/warnings.js';

/** @param {number} x */
const linear = (x) => x;

Expand Down Expand Up @@ -92,6 +96,8 @@ export function fly(
};
}

var slide_warning = false;

/**
* Slides an element in and out.
*
Expand All @@ -101,6 +107,13 @@ export function fly(
*/
export function slide(node, { delay = 0, duration = 400, easing = cubic_out, axis = 'y' } = {}) {
const style = getComputedStyle(node);

if (DEV && !slide_warning && /(contents|inline|table)/.test(style.display)) {
slide_warning = true;
Promise.resolve().then(() => (slide_warning = false));
w.transition_slide_display(style.display);
}

const opacity = +style.opacity;
const primary_property = axis === 'y' ? 'height' : 'width';
const primary_property_value = parseFloat(style[primary_property]);
Expand Down
Loading