Skip to content

Conversation

jycouet
Copy link
Contributor

@jycouet jycouet commented Oct 3, 2025

Here is the result:
image

Summary

  • a new file /src/lib/PlaygroundLayout.svelte to bring the Playground look and feel.
    • title: name of the playground
    • link: to the online playground
    • theme switch available
  • change files location to src/lib/playground

Copy link

changeset-bot bot commented Oct 3, 2025

🦋 Changeset detected

Latest commit: 432899c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sv Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link

pkg-pr-new bot commented Oct 3, 2025

Open in StackBlitz

npx https://pkg.pr.new/sveltejs/cli/sv@731
npx https://pkg.pr.new/sveltejs/cli/svelte-migrate@731

commit: 432899c

@manuel3108
Copy link
Member

Even if I have said different things in the past, i must say that I kinda like this. The only thing i would like to prevent code wise is to have the massive string in that source code file. Maybe extract it into it's own file, or read that template in from a text file or something similar. That's the only thing i would suggest.

@jycouet
Copy link
Contributor Author

jycouet commented Oct 12, 2025

It's good to have stuff step by step (also before, the idea wass less "designed" and too early).

I agree with you that this massive string is not great. I will look at moving it to a file ✌️
(For some reason, I know it can be a challenge to have this file not ignoed at publush time, old nightmare... Will check :D)

Copy link
Member

@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a source code perspective, this looks really good and intuitive. The problem is that we stumble upon #568 (or better: svelte-add/svelte-add#193)

This is the error im getting:

[plugin:vite-plugin-svelte:compile] D:/cli-temp/src/lib/PlaygroundLayout.svelte:56:51 Unterminated string constant
https://svelte.dev/e/js_parse_error
PlaygroundLayout.svelte:56:51
54 |          <span aria-hidden="true" style="margin-left:0.25em;"> ↗</span>
 55 |        </a>
 56 |        <button class="raised theme-toggle" onclick="{()" => setTheme(isDark ? 'light' : 'dark')}
                                                             ^
 57 |          aria-label="Toggle theme"
 58 |        &gt;

I'm not sure if / how we can solve this without #568 which still has quite a long way to go. Maybe we could make both properties component parameters which are initialized from +page.svelte. I think that's the only reasonable quickfix i can currently imagine.

@jycouet
Copy link
Contributor Author

jycouet commented Oct 18, 2025

Hooo, I'm confused!
It was not the case when it was not extracted into another file... I'll investigate.

@manuel3108
Copy link
Member

Probably because we did not need to parse the file? I think either the parsing or the printing is doing strang stuff, since we are currently not using the svelte compiler

@jycouet
Copy link
Contributor Author

jycouet commented Oct 18, 2025

Probably because we did not need to parse the file? I think either the parsing or the printing is doing strang stuff, since we are currently not using the svelte compiler

Yeah, so I updated the code to still use it (to have lang="ts" or not) + reduce the template as much as possible + doing a replace. Not that nice, but working workaround.
I added a few tests to make sure the output is ok.

Copy link
Member

@manuel3108 manuel3108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great now!

@manuel3108 manuel3108 merged commit 3d5f4de into main Oct 18, 2025
8 checks passed
@manuel3108 manuel3108 deleted the feat-playground-layout branch October 18, 2025 13:07
@github-actions github-actions bot mentioned this pull request Oct 18, 2025
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.

2 participants