New 'Hello World' page #934

rafaelcamaram · 2021-04-09T01:09:15Z

This PR aims basically to add a new 'Hello World' page as it was requested by #90 (created by @Rich-Harris).

Figma File by @vedam

Tests

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

CLI - Preview - (first step)

Default template - Preview

Skeleton template - Preview

Observation

I'm still performing my first baby steps with Svelte, so feel free to let me know if I can improve something 😛

…ions -. Applying it to the CLI as well

…leton

…s color needs to be dynamic based on the theme mode

benmccann · 2021-04-09T17:41:39Z

Wow, cool! This PR if very much appreciated!

My main question would be around the skeleton vs hello world app. I don't have a fully formed opinion on this yet. Some things to think about:

I wonder if it will become burdensome to maintain both
Should we combine the hello world template with https://github.com/sveltejs/kit/tree/master/examples/svelte-kit-demo somehow?
Will having both make things like svelte-adders harder? (CC @babichjacob)

Maybe we should go with the fleshed out version and have a svelte-adder (or remover/modifier 😉 ) that remove the extra files

Also, if you happen to want to do any more of this type of work you might be interested in #244 as well 😄

rafaelcamaram · 2021-04-09T18:07:21Z

Hey @benmccann, I'm really excited to know that you appreciated it :)

IMHO, I only applied the skeleton template because it was suggested originally on the issue but I believe we could follow what you mentioned keeping only the default one. If the user wants to simplify it and get the same files we have at the skeleton template, it'll be necessary to remove only 5 files (or use the svelte-adder, as you mentioned). What do you think? Can we remove it then?

Oh, and definitely, I'll take a look at #244 and dedicate some time to it! 😉

benmccann · 2021-04-09T19:58:47Z

Yeah, makes sense. It's probably a good idea to have both. The question in my mind would mostly be around how to accomplish it. Right now there's two copies of a lot of files like https://github.com/rafaelcamaram/kit/blob/camara/gh-90/packages/create-svelte/template-skeleton/README.md and we'd have to modify two copies of them everytime we want to make a change and I imagine they'd get out of sync and might become annoying to deal with

I'd want the other maintainers to weigh in on these questions, but I personally might suggest the following:

Just have the one template, but make the create-svelte script strip out all the stuff we don't want to create the skeleton. Add a test to make sure that script succeeds
Remove the SvelteKit demo project in favor of this new project. The demo project looks really outdated by comparison now and doesn't provide anything additional
Remove the CSS option and make it purely a svelte-adder thing. We can make create-svelte print a message pointing to svelte-adders. I doesn't make sense to me that create-svelte is responsible for creating Less and Sass templates, but not PostCSS/Tailwind which are probably more widely used in the Svelte community

Thanks again for all your help with this!

packages/create-svelte/template/src/lib/DarkModeToggle.svelte

packages/create-svelte/template/src/routes/blog.svelte

packages/create-svelte/template-skeleton/.gitignore

packages/create-svelte/template-additions/src/app.css

… handled only by svelte-adders

…ry svg files and fixing small design issue

rafaelcamaram · 2021-04-12T00:28:27Z

Hey @benmccann, I just applied all the suggestions you gave. Please, let me know if I misunderstood something or if there is something more to improve :)

Created a better way to allow the user to choose between both templates but using only a single one -- using modifications in order to build the skeleton one;
Removed the SvelteKit demo project -- do we need to update the docs as well?
Removed the support for CSS modifications from the CLI;
Applied the changes requested on PR comments;

…nsidering skeleton template

packages/create-svelte/cli/index.js

dummdidumm · 2021-04-12T07:04:48Z

packages/create-svelte/cli/index.js

 	const ts_response = await prompts({
 		type: 'confirm',
 		name: 'value',
 		message: 'Use TypeScript in components?',
 		initial: false
 	});
-	await add_typescript(target, ts_response.value);
-
-	const css_response = await prompts({


Personally I would have left that in it's really easy to setup, we don't even need to add much code modifications. Also, do we have any data how many people would choose what? I think people would opt for scss quite a bit just because many component libraries use it for their styling/theming.

Hm, I see your point.. The only drawback I see is that we would need to maintain 3 different styling files (I believe that's why @benmccann suggested removing it) but i's not a big deal for me -- it'd be easy to revert.

@dummdidumm @benmccann How should I move forward with it?

We chatted about this part for a bit and it sounds like @Rich-Harris and @pngwn also vote to take out the CSS stuff

packages/create-svelte/template/src/app.css

packages/create-svelte/template/src/lib/Counter.svelte

…rn and and scoping data-theme to :root

…gh-90

benmccann · 2021-04-12T22:33:47Z

packages/create-svelte/template/src/lib/HeaderNavigation.svelte

+		background-color: var(--accent-color);
+		transform: rotate(45deg);
+	}
+</style>


it'd probably be nice to add a new line to the end of this file

benmccann · 2021-04-12T22:33:58Z

packages/create-svelte/template/static/moon-icon.svg

@@ -0,0 +1,3 @@
+<svg width="22" height="22" viewBox="0 0 22 22" fill="none" xmlns="http://www.w3.org/2000/svg">
+    <path d="M13.0264 1.20543C10.2914 1.91848 8.27275 4.4053 8.27275 7.36365C8.27275 10.8782 11.1218 13.7273 14.6363 13.7273C17.5947 13.7273 20.0815 11.7086 20.7945 8.97365C20.9292 9.628 21 10.3058 21 11C21 16.5229 16.5229 21 11 21C5.47715 21 1 16.5229 1 11C1 5.47715 5.47715 1 11 1C11.6942 1 12.372 1.07075 13.0264 1.20543Z" stroke="#676778" stroke-width="2" stroke-linejoin="round"/>
+</svg>


it'd probably be nice to add a new line to the end of this file

benmccann · 2021-04-12T22:34:03Z

packages/create-svelte/template/static/sun-icon.svg

+    <path d="M4.75 20.5C5.44035 20.5 6 19.9404 6 19.25C6 18.5597 5.44035 18 4.75 18C4.05964 18 3.5 18.5597 3.5 19.25C3.5 19.9404 4.05964 20.5 4.75 20.5Z" fill="white" fill-opacity="0.6"/>
+    <path d="M1.75 13.25C2.44036 13.25 3 12.6904 3 12C3 11.3097 2.44036 10.75 1.75 10.75C1.05964 10.75 0.5 11.3097 0.5 12C0.5 12.6904 1.05964 13.25 1.75 13.25Z" fill="white" fill-opacity="0.6"/>
+    <path d="M4.75 6C5.44035 6 6 5.44035 6 4.75C6 4.05964 5.44035 3.5 4.75 3.5C4.05964 3.5 3.5 4.05964 3.5 4.75C3.5 5.44035 4.05964 6 4.75 6Z" fill="white" fill-opacity="0.6"/>
+</svg>


it'd probably be nice to add a new line to the end of this file

benmccann · 2021-04-12T22:37:51Z

packages/create-svelte/cli/modifications/add_typescript.js

 */
-export default async function add_typescript(cwd, yes) {
+export default async function add_typescript(cwd, yes, is_skeleton_template) {


there was a question about whether we could reverse this to start with the typescript template and make it remove_typescript. we have svelte-preprocess which should already offer the ability to strip out the typescript stuff and would make it so that we don't have to update this script at all when we change the template. using svelte-preprocess might be a big change to include in this PR, but it might be a step towards that to at least reverse the logic even if still done manually

dummdidumm · 2021-04-13T06:51:30Z

I think we should hold off on going further here until #989 or something similar is merged and rebuild upon that, else merge conflicts galore.

rafaelcamaram · 2021-04-13T22:54:49Z

Since we got several conflicts here, I thought it would be easier to create a new PR based on the new master.

New PR related to the new 'Hello World' templates: #1007

rafaelcamaram added 8 commits April 8, 2021 21:41

Adding new 'hello world' templates - both skeleton and default variat…

5137adb

…ions -. Applying it to the CLI as well

Fixing typo related to 'hello world' message pointing index route path

6b4be20

Adding changeset

5a4277b

Updating create-svelte lint command in order to consider template-ske…

574cb02

…leton

Removing unnecessary console.logs

c5df749

Moving Sun/Moon SVG code to asset SVG file

ca0ccea

Moving Increase/Decrease Counter SVG codes to asset SVG files

a0019e4

Reverting changes related to Increase/Decrease Counter icons since it…

a2722c4

…s color needs to be dynamic based on the theme mode

rafaelcamaram changed the title ~~New 'Hello World' page #90~~ New 'Hello World' page Apr 9, 2021

rafaelcamaram marked this pull request as draft April 9, 2021 01:58

Improving some points related to add_css and add_typescript file methods

d692fdc

rafaelcamaram marked this pull request as ready for review April 9, 2021 13:51

benmccann reviewed Apr 9, 2021

View reviewed changes

rafaelcamaram added 6 commits April 9, 2021 17:30

Removing skeleton template

68080f6

Fixing PR comments made by benmccann

3e43b5e

Removing svelte-kit-demo in favor of new hello world page

5f5523c

Removing built-in support for CSS/LESS/SCSS modifications (it will be…

0c71f4f

… handled only by svelte-adders

Adding is_skeleton_template to create-svelte CLI

815475d

Fixing merge conflicts with master

30968db

benmccann mentioned this pull request Apr 11, 2021

Update CSS for default template counter button #960

Closed

5 tasks

rafaelcamaram added 3 commits April 11, 2021 20:57

Fixing merge conflicts with master related to Counter.svelte

fb48e17

New Hello World page. Fixing add_typescript typos, removing unnecessa…

6221e23

…ry svg files and fixing small design issue

Applying prettier/lint

bad4fbc

Updating Hellow World eslint package json command in order to stop co…

b775c3c

…nsidering skeleton template

rafaelcamaram requested a review from benmccann April 12, 2021 00:37

benmccann requested a review from Rich-Harris April 12, 2021 02:14

This was linked to issues Apr 12, 2021

The Counter <button> in default template has color contrast issues #961

Closed

The best 'hello world' you've ever seen #90

Closed

dummdidumm reviewed Apr 12, 2021

View reviewed changes

packages/create-svelte/cli/index.js Outdated Show resolved Hide resolved

dummdidumm reviewed Apr 12, 2021

View reviewed changes

packages/create-svelte/template/src/app.css Outdated Show resolved Hide resolved

dummdidumm reviewed Apr 12, 2021

View reviewed changes

packages/create-svelte/template/src/lib/Counter.svelte Outdated Show resolved Hide resolved

rafaelcamaram added 2 commits April 12, 2021 09:38

Applying suggestions made by dummdidumm regarding CSS classname patte…

d90c015

…rn and and scoping data-theme to :root

Merge branch 'master' of github.com:sveltejs/kit into camara/sveltejs…

447f4f6

…gh-90

rafaelcamaram requested review from babichjacob and dummdidumm April 12, 2021 13:13

Updating CLI message related to addon possibilities

78b8ece

benmccann mentioned this pull request Apr 12, 2021

Add command to navigate to and run svelte-kit-demo #881

Closed

Rich-Harris mentioned this pull request Apr 12, 2021

Simplify create-svelte #987

Closed

benmccann reviewed Apr 12, 2021

View reviewed changes

rafaelcamaram mentioned this pull request Apr 13, 2021

New 'Hello World' templates #1007

Closed

1 task

rafaelcamaram closed this Apr 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New 'Hello World' page #934

New 'Hello World' page #934

rafaelcamaram commented Apr 9, 2021 •

edited

Loading

benmccann commented Apr 9, 2021 •

edited

Loading

rafaelcamaram commented Apr 9, 2021

benmccann commented Apr 9, 2021 •

edited

Loading

rafaelcamaram commented Apr 12, 2021 •

edited

Loading

dummdidumm Apr 12, 2021

rafaelcamaram Apr 12, 2021 •

edited

Loading

benmccann Apr 12, 2021

benmccann Apr 12, 2021

benmccann Apr 12, 2021

benmccann Apr 12, 2021

benmccann Apr 12, 2021

dummdidumm commented Apr 13, 2021

rafaelcamaram commented Apr 13, 2021

New 'Hello World' page #934

New 'Hello World' page #934

Conversation

rafaelcamaram commented Apr 9, 2021 • edited Loading

Tests

CLI - Preview - (first step)

Default template - Preview

Skeleton template - Preview

Observation

benmccann commented Apr 9, 2021 • edited Loading

rafaelcamaram commented Apr 9, 2021

benmccann commented Apr 9, 2021 • edited Loading

rafaelcamaram commented Apr 12, 2021 • edited Loading

dummdidumm Apr 12, 2021

Choose a reason for hiding this comment

rafaelcamaram Apr 12, 2021 • edited Loading

Choose a reason for hiding this comment

benmccann Apr 12, 2021

Choose a reason for hiding this comment

benmccann Apr 12, 2021

Choose a reason for hiding this comment

benmccann Apr 12, 2021

Choose a reason for hiding this comment

benmccann Apr 12, 2021

Choose a reason for hiding this comment

benmccann Apr 12, 2021

Choose a reason for hiding this comment

dummdidumm commented Apr 13, 2021

rafaelcamaram commented Apr 13, 2021

rafaelcamaram commented Apr 9, 2021 •

edited

Loading

benmccann commented Apr 9, 2021 •

edited

Loading

benmccann commented Apr 9, 2021 •

edited

Loading

rafaelcamaram commented Apr 12, 2021 •

edited

Loading

rafaelcamaram Apr 12, 2021 •

edited

Loading