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: Display directive #14795

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

adiguba
Copy link
Contributor

@adiguba adiguba commented Dec 21, 2024

WIP : Another possible solution for #9976

This add a new #display directive for DOM elements.
This directive will allow to show/hide an element using the Svelte transition API, without destroying it.

<div transition:slide #display={visible}>
   ...
</div>

=> When visible is "falsy", the element will be hidden using a display: none !important.
Otherwise it will be show either by removing the display style, or setting the value of style:display.

Note : I known that #display={...] is not usual for directive but I think it's more expressive...
In any case we could replace it with something like svelte:display={...}

It's still a work in progress, and need some works that I can do if the syntax of this PR is accepted...

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented Dec 21, 2024

⚠️ No Changeset found

Latest commit: 272f7d4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-svelte-14795-svelte.vercel.app/

this is an automated message

Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@14795

@Thiagolino8
Copy link

Thiagolino8 commented Dec 21, 2024

Wouldn't it be possible for this to be a block?

Kinda like a vue v-show directive equivalent?

{#show visible}
  <p transition:slide>hello world</p>
{/show}

@adiguba
Copy link
Contributor Author

adiguba commented Dec 21, 2024

Wouldn't it be possible for this to be a block?

It could only work by setting the style display property, so it need an element.

And yes it's an equivalent to Vue v-show

@kran6a
Copy link

kran6a commented Dec 21, 2024

Wouldn't it be possible for this to be a block?

Kinda like a vue v-show directive equivalent?

{#show visible}
  <p transition:slide>hello world</p>
{/show}

I would prefer a block too, it works basically like an #if and making it a block means we can place multiple HTML elements inside. It also means users don't have to learn new syntax. Components within the block would get passed the corresponding style prop (e.g: style="display:none!important"). If style prop is already present it would upsert the display property accordingly.

<div #display={visible}></div> seems odd and would introduce new syntax (with its corresponding docs).

Also I don't think this feature is important enough to deserve its own syntax as we can already do this:

<div style:display={visible}></div>

Pair that with a getter, a setter and factory pattern to turn it into something like:

<script>
  const handler = visibility_handler("flex", false); //Create an object with a setter and a getter that switches state between "flex" and "none!important" with an initial value of "none!important" as indicated by the second argument
</script>
`<div style:display={handler.visible}></div>`

Which allows you to do things like:

handler.toggle();
handler.visible = false; //assignment is captured via its setter. handler.visible getter now returns "none!important"
handler.visible = true; //assignment is captured via its setter. handler.visible now returns "flex" as it was memoized
handler.visible = "inline" //assignment is captured via its setter. The initial "flex" value is overridden
handler.hide();
handler.show();

@adiguba
Copy link
Contributor Author

adiguba commented Dec 21, 2024

I would prefer a block too, it works basically like an #if and making it a block means we can place multiple HTML elements inside.

But in order to hide the content, we need an element on which to set display:none.
This will cause problems if the block contains text, @html or Components (there no way to pass a style to any component)...

=> How do you hide that :

{#display visible}
   Hello World !
{/display}

Also I don't think this feature is important enough to deserve its own syntax as we can already do this:

<div style:display={visible}></div>

I known that style:display can be used to show/hide an element, but this does not execute the transitions.

Updated example :
https://svelte.dev/playground/hello-world?version=pr-14795#H4sIAAAAAAAACpVU7Y7bIBB8Feqr5ER3OSdVVVU-x1XVP-0zNP2B8dpCwWDBOr2rlXcvGLCT-5BaISswzO4MG9gxkbSDJE--gxCK_FZa1GQFNUeo18ld0nABJsl_jgk-9Y7nAIuHqK99f29OINBhFTXwGs6URJBo0ySFYZr3WB7kAXnXK41kJEbwGsiZNFp1JPVhGWoqDUeuZHpB_qYGm0sH6n0W1kErfXBUAUhO3PBKANmT9wYpwgr1AGu7XWSLAzuKakBUkijJBGfH_bha78sQvH8XJucSVdvabHFjjBtF5uOnbEXNT4QJasw-bTWv0-mUDp0mB7zhjZ-MdhaTnT3kiTG8EoodU7LUIJ9qVEauZcdKZDNYZIvUmPFmyryAl0ZqbnpBn_zqH4RJjFiOvsg-d3Ih-Zq4wScB-X87uAqbbXyRgxB5KpWE9BVLm00MOCSOc0jeNjkjYRSTogcwXJPRB4ekOZnMPniwo7rlMic76AgdUAW4p3XNZZuTz_0j2X3qHyfc_-v2u3c35UVeB4Z4N90gdHYDYcOUGDpprEqj4xeJtJ_EwxLhETdU8NZaYuCqMQs72cl51K2UrkHbaGvRKFtucsMYC4kqyo6ttgWtc3LT7Nx4frQPs2qQsK9sLp5tAM5LkrsneL57o5FcP-TrZvJi742G4h4-c9zl2W_dm3d776FpgOFqtSb7Mp7b5jFIjpZuAH84jRMVgTIlur29I7vtdrv2fA04aEkCQQDVc9Bx7cvrf571maIqxymfaxjl9eXCKrpp7Kk2hv-BnHwMBb0o5XUZf53_AodSc4G5BQAA

@Leonidaz
Copy link

Leonidaz commented Dec 21, 2024

Wouldn't it be possible for this to be a block?

Kinda like a vue v-show directive equivalent?

{#show visible}
 <p transition:slide>hello world</p>
{/show}

Wouldn't it be possible for this to be a block?

It could only work by setting the style display property, so it need an element.

And yes it's an equivalent to Vue v-show

This is really sweet! Hope in some shape or form this can be incorporated.

I also wonder if this could be a block, this way it would be intuitive and basically only way to do it, vs if someone used and if block and the directive.

I think a block could work with only the top level elements, where every top level element would be affected just like with the regular {#if} block.

Components at the top level would NOT be allowed but could be nested inside elements.

I think it can also play well with the transition |global option with nested blocks.

I think it would make sense to use the {#if} block instead of creating another one as the idea is basically to "tell" the {#if} block to perform a certain action and instead of removing / adding / mounting / unmounting, it should perform a different action.

Please read to the end as this has been more like a discovery journey.
Or just skip to the end. sorry, looks like github doesn't do links inside comments

<script>
  import { fade } from 'svelte/transition';
  
  let signal = $state(true);
</script>

<!-- toggle #if condition -->
<button onclick={() => signal = !signal}>Fade in / out</button>

<!-- short form with functions predefined -->
{#if:transition=[in, out] signal}
   <!-- every top element will run through in and out callbacks, components disallowed at the top -->
  <div transition:fade>fades in and out</div>
{/if}

<!-- shortest form with a function that returns a tuple with for in and out  -->
{#if:transition=display() signal}
   <!-- every top element will run through in and out callbacks, components disallowed at the top -->
  <div transition:fade>fades in and out</div>
{/if}

<!-- long form inlined functions, in out functions run for every top element -->
{#if:transition=[node => node.removeAttr('style'), node => node.style.display = 'none')] signal}
   <!-- every top element will run through in and out callbacks, components disallowed at the top -->
  <div transition:fade>fades in and out</div>
{/if}

The syntax for {#if could also be more like event handling, if Svelte ends up adding other #if actions, e.g.

{#if:on:transition=[in, out] signal}

Svelte could provide a super basic, like above, display if action with an import svelte/transition/actions but people would be free to create their own if they want more custom logic. One of the actions could be to remove what's inside if, how it currently works. The passed in node to callbackups, could also be a fake / proxy that could limit what could be done.

with svelte provided actions:

<script>
  import { fade } from 'svelte/transition'
  import { display } from 'svelte/transition/actions';
  
  let signal = $state(true);
</script>

{#if:transition=display() signal}
   <!-- every top element will run through in and out callbacks, components disallowed at the top -->
  <div transition:fade>fades in and out</div>
{/if}

Simplification

But, maybe even simpler, {#if} is already aware of the fact that it needs to wait on transitions. And so, it would make sense just to tell it what action to perform instead of removing all nodes when transitions finish - based on the state of the if condition true or false. The other, arguably, very useful feature that is gained is that even without any transitions, the if-action can be performed on the top elements, e.g. do something else like hide instead of removing from the DOM.

{#if:action=[truthy, falsy]}
{/if}

a more fleshed out example, and actually, it probably just makes sense to use the same syntax as for the transition: directive and provided / custom functions:

<script>
  import { fade } from 'svelte/transition'
  import { display } from 'svelte/transition/actions';
  // display or any custom function that has to return a tuple of functions for truthy and falsy
  // display() => [(node: HTMLElement) => void, (node: HTMLElement) => void]
  
  // example of what `display` could return: [node => node.removeAttr('style'), node => node.style.display = 'none')] 
  
  let signal = $state(true);
</script>

{#if:action:display signal}
  <div transition:fade>fades in and out</div>
{/if}

And to reiterate:

  • All top-level elements would be affected just like with the regular {#if} block.
  • The truthy and falsy functions provided by an action will run for every top-level element
  • Components at the top level are NOT allowed but could be nested inside elements at any other level.
  • All elements that don't have a transition: specified will be affected by the action
  • If there aren't any elements with transition:, the action will simply be performed on the top-level elements immediately.
  • Overall, allowing if blocks to perform actions other than removing elements / unmounting components

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants