From d36ce0ac173ff8574bcfdfb7a37a373af08b6d6c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 7 Jan 2025 12:25:11 -0500 Subject: [PATCH 1/3] feat: warn on using `slide` transition with table elements --- .changeset/silver-humans-yawn.md | 5 +++++ .../docs/98-reference/.generated/client-warnings.md | 10 ++++++++++ .../svelte/messages/client-warnings/warnings.md | 8 ++++++++ packages/svelte/src/internal/client/warnings.js | 12 ++++++++++++ packages/svelte/src/transition/index.js | 13 +++++++++++++ 5 files changed, 48 insertions(+) create mode 100644 .changeset/silver-humans-yawn.md diff --git a/.changeset/silver-humans-yawn.md b/.changeset/silver-humans-yawn.md new file mode 100644 index 000000000000..e2743410791b --- /dev/null +++ b/.changeset/silver-humans-yawn.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: warn on using `slide` transition with table elements diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index 5218ec4cb1f8..c8fede681c3c 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -222,3 +222,13 @@ 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_table + +``` +The `slide` transition does not work correctly with elements with `display: %value%` +``` + +The [slide](/docs/svelte/svelte-transition#slide) transition works by animating the `height` of the element. Setting `height` to a value smaller than the element's natural height has no effect if the element's [`display`](https://developer.mozilla.org/en-US/docs/Web/CSS/display) style starts with `table` (which is the default state of table elements such as ``). + +Consider using a `flex` or `grid` layout instead. diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 21db506dc9da..7fda90c326b3 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -186,3 +186,11 @@ 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_table + +> The `slide` transition does not work correctly with elements with `display: %value%` + +The [slide](/docs/svelte/svelte-transition#slide) transition works by animating the `height` of the element. Setting `height` to a value smaller than the element's natural height has no effect if the element's [`display`](https://developer.mozilla.org/en-US/docs/Web/CSS/display) style starts with `table` (which is the default state of table elements such as ``). + +Consider using a `flex` or `grid` layout instead. diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 24d86e2e0b35..98fb47124df0 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -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 with elements with `display: %value%` + * @param {string} value + */ +export function transition_slide_display_table(value) { + if (DEV) { + console.warn(`%c[svelte] transition_slide_display_table\n%cThe \`slide\` transition does not work correctly with elements with \`display: ${value}\`\nhttps://svelte.dev/e/transition_slide_display_table`, bold, normal); + } else { + console.warn(`https://svelte.dev/e/transition_slide_display_table`); + } } \ No newline at end of file diff --git a/packages/svelte/src/transition/index.js b/packages/svelte/src/transition/index.js index 41231062115d..9e4274fa7ba1 100644 --- a/packages/svelte/src/transition/index.js +++ b/packages/svelte/src/transition/index.js @@ -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; @@ -92,6 +96,8 @@ export function fly( }; } +var slide_warning = false; + /** * Slides an element in and out. * @@ -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 && style.display.startsWith('table-') && !slide_warning) { + slide_warning = true; + Promise.resolve().then(() => (slide_warning = false)); + w.transition_slide_display_table(style.display); + } + const opacity = +style.opacity; const primary_property = axis === 'y' ? 'height' : 'width'; const primary_property_value = parseFloat(style[primary_property]); From 8c33015186729aaf02b823d7b2c37255c5753e2d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 7 Jan 2025 12:52:54 -0500 Subject: [PATCH 2/3] more generic --- .../docs/98-reference/.generated/client-warnings.md | 8 +++++--- packages/svelte/messages/client-warnings/warnings.md | 8 +++++--- packages/svelte/src/internal/client/warnings.js | 6 +++--- packages/svelte/src/transition/index.js | 10 ++++++++-- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index c8fede681c3c..cba8343108eb 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -223,12 +223,14 @@ 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_table +### transition_slide_display ``` The `slide` transition does not work correctly with elements with `display: %value%` ``` -The [slide](/docs/svelte/svelte-transition#slide) transition works by animating the `height` of the element. Setting `height` to a value smaller than the element's natural height has no effect if the element's [`display`](https://developer.mozilla.org/en-US/docs/Web/CSS/display) style starts with `table` (which is the default state of table elements such as ``). +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: -Consider using a `flex` or `grid` layout instead. +- `display: inline` (which is the default for elements like ``), and its variants like `inline-block`, `inline-flex` and `inline-grid` +- `display: table` and `table-[name]`, which are the defaults for elements like `` and `` +- `display: contents` diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 7fda90c326b3..6493501093e4 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -187,10 +187,12 @@ 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_table +## transition_slide_display > The `slide` transition does not work correctly with elements with `display: %value%` -The [slide](/docs/svelte/svelte-transition#slide) transition works by animating the `height` of the element. Setting `height` to a value smaller than the element's natural height has no effect if the element's [`display`](https://developer.mozilla.org/en-US/docs/Web/CSS/display) style starts with `table` (which is the default state of table elements such as ``). +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: -Consider using a `flex` or `grid` layout instead. +- `display: inline` (which is the default for elements like ``), and its variants like `inline-block`, `inline-flex` and `inline-grid` +- `display: table` and `table-[name]`, which are the defaults for elements like `
` and `` +- `display: contents` diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 98fb47124df0..67222dbebdb4 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -171,10 +171,10 @@ export function state_proxy_equality_mismatch(operator) { * The `slide` transition does not work correctly with elements with `display: %value%` * @param {string} value */ -export function transition_slide_display_table(value) { +export function transition_slide_display(value) { if (DEV) { - console.warn(`%c[svelte] transition_slide_display_table\n%cThe \`slide\` transition does not work correctly with elements with \`display: ${value}\`\nhttps://svelte.dev/e/transition_slide_display_table`, bold, normal); + console.warn(`%c[svelte] transition_slide_display\n%cThe \`slide\` transition does not work correctly with elements with \`display: ${value}\`\nhttps://svelte.dev/e/transition_slide_display`, bold, normal); } else { - console.warn(`https://svelte.dev/e/transition_slide_display_table`); + console.warn(`https://svelte.dev/e/transition_slide_display`); } } \ No newline at end of file diff --git a/packages/svelte/src/transition/index.js b/packages/svelte/src/transition/index.js index 9e4274fa7ba1..d897be378be1 100644 --- a/packages/svelte/src/transition/index.js +++ b/packages/svelte/src/transition/index.js @@ -108,10 +108,16 @@ var slide_warning = false; export function slide(node, { delay = 0, duration = 400, easing = cubic_out, axis = 'y' } = {}) { const style = getComputedStyle(node); - if (DEV && style.display.startsWith('table-') && !slide_warning) { + if ( + DEV && + !slide_warning && + (style.display.includes('table') || + style.display.includes('inline') || + style.display === 'contents') + ) { slide_warning = true; Promise.resolve().then(() => (slide_warning = false)); - w.transition_slide_display_table(style.display); + w.transition_slide_display(style.display); } const opacity = +style.opacity; From 4321c1e05650d6da2f89a57e9a722644cc737376 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 7 Jan 2025 12:54:01 -0500 Subject: [PATCH 3/3] more generic --- .../docs/98-reference/.generated/client-warnings.md | 2 +- packages/svelte/messages/client-warnings/warnings.md | 2 +- packages/svelte/src/internal/client/warnings.js | 4 ++-- packages/svelte/src/transition/index.js | 8 +------- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/documentation/docs/98-reference/.generated/client-warnings.md b/documentation/docs/98-reference/.generated/client-warnings.md index cba8343108eb..284e9a7c3e57 100644 --- a/documentation/docs/98-reference/.generated/client-warnings.md +++ b/documentation/docs/98-reference/.generated/client-warnings.md @@ -226,7 +226,7 @@ To resolve this, ensure you're comparing values where both values were created w ### transition_slide_display ``` -The `slide` transition does not work correctly with elements with `display: %value%` +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: diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 6493501093e4..943cf6f01f4f 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -189,7 +189,7 @@ To resolve this, ensure you're comparing values where both values were created w ## transition_slide_display -> The `slide` transition does not work correctly with elements with `display: %value%` +> 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: diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 67222dbebdb4..78d457f48f55 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -168,12 +168,12 @@ export function state_proxy_equality_mismatch(operator) { } /** - * The `slide` transition does not work correctly with elements with `display: %value%` + * 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 with elements with \`display: ${value}\`\nhttps://svelte.dev/e/transition_slide_display`, bold, normal); + 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`); } diff --git a/packages/svelte/src/transition/index.js b/packages/svelte/src/transition/index.js index d897be378be1..68c460c870c4 100644 --- a/packages/svelte/src/transition/index.js +++ b/packages/svelte/src/transition/index.js @@ -108,13 +108,7 @@ var slide_warning = false; export function slide(node, { delay = 0, duration = 400, easing = cubic_out, axis = 'y' } = {}) { const style = getComputedStyle(node); - if ( - DEV && - !slide_warning && - (style.display.includes('table') || - style.display.includes('inline') || - style.display === 'contents') - ) { + 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);