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: bring over examples #294

Merged
merged 16 commits into from
Oct 9, 2024
Merged

feat: bring over examples #294

merged 16 commits into from
Oct 9, 2024

Conversation

dummdidumm
Copy link
Member

  • copy examples over from the Svelte repo, they now live here
  • migrate examples to Svelte 5 syntax, remove some deprecated examples in the process
  • add select to playground from which examples can be chosen

closes #27

Side note: I'm a bit wary of all these 308 redirects we're doing - if we ever decide we want to reuse those pages for something else, we can't, because caches all over the world will still redirect.

Copy link

vercel bot commented Oct 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
omnisite ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2024 5:09pm

@Rich-Harris
Copy link
Member

We can always do 307s instead?

@dummdidumm
Copy link
Member Author

Yeah I'd prefer that honestly

@dummdidumm
Copy link
Member Author

During implementing this I stumbled upon (what I think is) a SvelteKit bug:
With this structure

/api
  [id].json/+server
  examples.json/+server

and [id].json fetching examples.json, the server is fine at dev time but crashes in prod. The manifest seems to not properly resolve, instead of fetching the prerendered examples.json it recursively calls itself in [id].json. Worked aroud it by moving the call into a subdirectory

@@ -0,0 +1,7 @@
<script>
let src = '/tutorial/image.gif';
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't work — cross-origin isolation gremlins strike again

Copy link
Member Author

Choose a reason for hiding this comment

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

... and this time it's impossible to get it working because we can't add the relaxed attribute on the iframe for security reasons.

I really think we need to make navigations to the SvelteKit tutorial full page reloads, and navigations away from it, too, and only set the headers on the SvelteKit tutorial. That would solve all this iframe/img/audio/video bullshit we had to deal with the last days. I don't expect people to quickly jump back and forth between that tutorial and something else, so it will be fine in practise.

@Rich-Harris
Copy link
Member

Glad to have this wired up. We will need to audit all the examples closely though. Some don't work, because they make network requests that fail, some are quite dated/strange, some are partially migrated (e.g. the clock example refers to $: in a comment, even though the code now uses $derived), some are showcasing legacy stuff like createEventDispatcher, some are just plain ugly.

A larger question is 'what are these examples for?' Is it weird that many of them are taken directly from the tutorial (while others were taken directly from the tutorial but have since fallen out of date)? If we think duplicating content between tutorial and examples is desirable, then should we formalise that and turn all the app-b directories into examples automatically? (Personally I don't think we should have these duplicates, it's weird. It clutters the search, or would if examples were included in search, and removes the context afforded by the exercise's text and its siblings.) Right now the list is neither concise nor exhaustive; it's a flabby in-between mish-mash. It would be good to develop a clear idea about what belongs here. (None of this is a criticism of this PR, of course — all this has long been the case.)

UI-wise I don't think we're quite there yet. It's a bit weird to have the text duplicated...


image

...and it gets truncated quite easily. It also doesn't match the styles of the neighbouring buttons, especially on Firefox where <select> elements are hard to style:


image

I was envisaging something more like this (it could still be backed by a <select>, albeit an invisible one):


image

Ideas:

  • in addition to examples, the dropdown menu has a 'Create new' (or similar wording) at the top, which creates a completely blank playground called Untitled
  • the first edit to an example would change the title from Hello world to Hello world (edited), and cause the red dot to appear
  • edting the title before editing files would also cause the red dot to appear; subsequent file edits would not cause a title change
  • selecting an example could pop up a 'unsaved changes, are you sure?' dialog if you have changes (though do we need this, if the state is preserved in the hash and you can just go back?)
  • examples should be included in search

@Rich-Harris
Copy link
Member

I stumbled upon (what I think is) a SvelteKit bug

gah. would be good to get that repro'd and filed if you have the bandwidth

@dummdidumm
Copy link
Member Author

dummdidumm commented Oct 9, 2024

  • move to burger menu to the left
  • add (edited) logic
  • add "Create new" option
  • included examples in search (with low priority, the titles match up too good for things you search and you likely don't want examples getting in your way all the time)

Didn't do the other changes because:

  • editing the filename making the save icon appear is a bit weird because the icon indicates how many files have changed, but no file has changed. We should postpone tweaking that
  • selecting an example causing a popup: as you say you get a hash link so I think it's unnecessary

As for vesting content: Should I remove all examples except hello world so we can merge the ground-work and then iterate on examples separately?

gah. would be good to get that repro'd and filed if you have the bandwidth

sveltejs/kit#12778

@Rich-Harris
Copy link
Member

super confused by the deploy failure. it's complaining about a bad link that doesn't exist AFAICT

@Rich-Harris
Copy link
Member

weiiirrddd, the link is there in the prod build but not in dev. investigating

@Rich-Harris
Copy link
Member

Should I remove all examples except hello world so we can merge the ground-work and then iterate on examples separately?

We can leave them in for now but I'll open a separate issue so we remember to sort through them

@dummdidumm
Copy link
Member Author

good to go from my side then

@Rich-Harris Rich-Harris merged commit 9ee57ca into main Oct 9, 2024
3 checks passed
@Rich-Harris Rich-Harris deleted the examples branch October 9, 2024 18:24
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.

Examples
2 participants