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

Changing tutorial - HTML and CSS order and button style #6213

Merged
merged 40 commits into from
Apr 22, 2021
Merged

Changing tutorial - HTML and CSS order and button style #6213

merged 40 commits into from
Apr 22, 2021

Conversation

babakfp
Copy link
Contributor

@babakfp babakfp commented Apr 20, 2021

Hi. The result of changes:
Screenshot 2021-04-19 181107

BabakFP added 4 commits April 19, 2021 17:48
- Also making the button simple and prettier.
- Also making the button simple and prettier.
@dummdidumm
Copy link
Member

What was the motivation for these changes? Also, if the html/css order is changed in one place, it should be changed in all places of the tutorial to be consistent - but I'm not sure that's something we want at this point, at least there needs to be some concensus if that's a desired change.

@babakfp
Copy link
Contributor Author

babakfp commented Apr 20, 2021

Why I changed the HTML and CSS order?

Because there is nothing related to CSS in this tutorial. So the target is the button element and users must see it first. in prev code, the user should scroll to see the HTML code but now the user can see it on the first look.

What about styling?

The old button was ugly and distracting. It can be a good choice if we remove all the styles because this tutorial isn't related to CSS and [+] it can be less distracting to the user.

@dummdidumm
Copy link
Member

Changing it because "it's more important" will be confusing. The purpose of the tutorial is to get familiar with Svelte in general, and switching up script/style/html order on a case-by-case-basis does not help there. That's why we need to be consistent.
I could live with some style changes though, not sure how the other maintainers feel about that. But it definetly needs some kind of styling, aesthetics matter, too.

@babakfp
Copy link
Contributor Author

babakfp commented Apr 20, 2021

Agree with consistency and having a good style.
Not a while ago if you were asked me what to do about styling, I would say that apply a global style for all the tutorial elements without the user can access them. So by doing this we can have pretty content without writing a single line style and distracting the user.
But now I say it isn't a good idea!
The standard ordering is this right?:

<script></script>
<p>Hello, World!</p>
<style></style>

But in the tutorial, we do not comply with it. So I think the best decision is to correct the ordering in all the tutorials and the examples and anywhere else.

@dummdidumm
Copy link
Member

What the standard is is not totally fixed, prettier-plugin-svelte uses this order by default, and some recent poll showed that the majority (but not by a very huge margin) uses that order, too.

@babakfp
Copy link
Contributor Author

babakfp commented Apr 21, 2021

@dummdidumm
In React, JS comes first and JSX second, and CSS in a separate file. In Vue, HTML comes first and JS second, and CSS at last.
It doesn't make sense that CSS be at the top of the order. Remains JS and HTML. I think it's best if JS comes after the HTML because:

  • We can use a variable before declaring it right? Having the HTML at the top is the same.
  • JS is more complex and harder than HTML, and hard to have in mind.
  • Most of the time we work with JS, well we need to have easy access to it.
  • HTML and CSS are like a couple, and JS like an outsider. (We learn HTML and CSS at the same time and later we learn JS).
    I don't know about other developers but I'm using the ordering, that I mentioned about it, everywhere. If most people use this ordering, then it's good to be consistent in the tutorials too.

@dummdidumm
Copy link
Member

Script comes first both in the current Svelte tutorial and the default formatting, so I vote for script-html-style

@babakfp
Copy link
Contributor Author

babakfp commented Apr 21, 2021

Hi
Yes, I agree. I will do some commits and fix the ordering on some tutorials and examples. Is it ok?

@babakfp
Copy link
Contributor Author

babakfp commented Apr 21, 2021

Oh God, I didn't know that the new commit going to affect this pull request.

BabakFP and others added 15 commits April 21, 2021 10:20
- Changing paragraph content.
- Changing comment.
- Changing paragraph content.
- Changing paragraph content.
- Changing paragraph content.
- Changing paragraph content.
- Format CSS
- Format CSS
- Format CSS
@babakfp
Copy link
Contributor Author

babakfp commented Apr 21, 2021

All done.
I didn't use Prettier or something like that.

I Just changed:

  • HTML and CSS order
  • CSS format
  • Removed the last empty line
  • 'Hello world!' to 'Hello, World!'

@babakfp
Copy link
Contributor Author

babakfp commented Apr 22, 2021

I have a question, why the tutorials and the examples are the same? I mean 90 of them are duplicates!.
https://svelte.dev/tutorial
https://svelte.dev/examples

@dummdidumm
Copy link
Member

I don't know honestly, but I guess because not everyone will look at the tutorial so there needs to be some overlap.

Thanks for rearranging all these! I see however that you do some slight adjustments along the way, like changing hello {world} to Hello, {world} . Could you revert these small adjustments? Otherwise it's hard to review this PR. I'd like this one to be purely about reordering (although the initial intent of this PR was totally different 😄 ). Afterwards, you could propose another PR with your style adjustments.

@babakfp
Copy link
Contributor Author

babakfp commented Apr 22, 2021

Sure.

@babakfp
Copy link
Contributor Author

babakfp commented Apr 22, 2021

Ok, all done.
We seriously need a code formater. We don't have a good code formater but, Prettier is better than nothing. In the next pull request can we add the prettier and the config file?

@dummdidumm
Copy link
Member

Thanks!
Formatting the whole repository is out of scope for now, for historic reasons.

@dummdidumm dummdidumm merged commit ec25407 into sveltejs:master Apr 22, 2021
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.

3 participants