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

New hello world #1014

Merged
merged 59 commits into from
Apr 18, 2021
Merged

New hello world #1014

merged 59 commits into from
Apr 18, 2021

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Apr 14, 2021

Following up @rafaelcamaram's work in #1007 with a few changes:

  • make it possible to do pnpm dev inside the template, for easier developing
  • fix accessibility of welcome image
  • use spring physics for the counter (transitions don't really work here)
  • ensure that hero image has dimensions set to prevent layout popping
  • use localStorage mechanism for dark mode, use CSS to swap welcome image
  • update welcome images (@vedam — we're currently using pngs ripped straight out of the figma document, but the light/dark mode welcome images don't line up perfectly, and it looks like the light mode one has some scaling artifacts. Do you happen to have originals handy?
  • populate about page with something
  • populate blog section with something

It would also be cool if this template was the demo app referred to in #639. To that end we may want to test/showcase various features:

  • dynamic endpoints
  • prerendering
  • non-hydrated pages
  • etc

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
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

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

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

@benmccann
Copy link
Member

Looks like you need to run prettier on create-svelte

@benmccann
Copy link
Member

lgtm. thanks @rafaelcamaram for blazing the trail here!

@vedam
Copy link
Member

vedam commented Apr 14, 2021

  • update welcome images – @vedam .... Do you happen to have originals handy?

I'll look for them asap.

@benmccann
Copy link
Member

Perhaps a bit off-topic, but the thing that's always bugged me about create-svelte is that it dumps the files in your current directory. I'd like it if it asked you for your project name and created a new directory and put the files there

@Rich-Harris
Copy link
Member Author

Thanks @vedam — have made those changes:

image

image

I did wonder about the deferred transition example — I was worried that it might be a bit too much for an introductory page, since it's quite advanced stuff that isn't totally obvious on first glance.

We're tracking the API stuff on sveltejs/api.svelte.dev#2. Once that's done, next step will be to figure out a way to deploy this demo app with the various adapters — it can take the place of https://github.com/sveltejs/kit/tree/master/examples/svelte-kit-demo (or this app could go there, and the create-svelte build process could read from that directory instead of its own templates/default).

@@ -0,0 +1,89 @@
import fetch from 'node-fetch';
Copy link
Member

Choose a reason for hiding this comment

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

I'm still a bit unsure about hitting a real api. Couldn't we do a mock here instead and write to local storage? If everyone else uses the db interface we can showcase these things just the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can't make an app that works without JS if you rely on localStorage

@dummdidumm
Copy link
Member

dummdidumm commented Apr 17, 2021

A general thought: should the the app be enhanced with a couple of comments explaining the various features showcased here with a link to the docs? Like "the load function can be used to fetch data. See -link- for more info". Else if someone jumps right in without reading the docs first it could be a little overwhelming.

@Rich-Harris
Copy link
Member Author

Yep, added some docs, and we can certainly add more

@Rich-Harris
Copy link
Member Author

API is rigged up, so I'll merge this in even though there's doubtless room for improvement. Next stop after this is released will be to get it deployed with the various different adapters

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.

6 participants