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

Update Transition Tutorial #2792

Closed
wants to merge 1 commit into from
Closed

Update Transition Tutorial #2792

wants to merge 1 commit into from

Conversation

Charca
Copy link
Contributor

@Charca Charca commented May 17, 2019

Description

I was following the Tutorial and I've noticed this really minor missing piece. The fade import was replaced with fly in exercise 10.b, but it needs to be re-added for 10.c to work correctly.

The `fade` import was removed in the previous chapter, it needs to be re-added for the example to work correctly.
@Conduitry
Copy link
Member

I don't think it's completely unreasonable to expect that people would have to also edit the imports when following a tutorial. I'm not sure what's the most pedagogically sound thing to have in the 'before' part of this page. It might be valuable to teach people here that they do have to import the transitions they want to use.

@varholak-peter
Copy link
Contributor

It ultimately depends on whether we want to have the tutorial as a continuous flow or self-contained into sections or even individual lessons.
If google redirects me to this specific lesson I might not know where to import the fade from. And I have to go into the previous lesson to learn that something like that is available to me.

@Charca
Copy link
Contributor Author

Charca commented May 17, 2019

This missing import stood out to me because this exercise was the only one that could not be completed by following the instructions only. You needed to know where to grab the fade module from.

I like the fact that each exercise in the tutorial can be completed by just following the instructions. This one just felt inconsistent with the rest of them.

@Rich-Harris
Copy link
Member

I agree with that point, but I think the fix is to make the instructions more explicit, so I've done that: 0d7f6fb Will close this PR, thank you

colincasey added a commit to colincasey/svelte that referenced this pull request May 28, 2019
* master: (87 commits)
  -> v3.4.3
  always add raw property to text nodes
  flesh out in/out transition tutorial chapter (sveltejs#2792)
  code style
  fix test
  fix tests
  Fix CRUD example to allow changing input values.
  Fixes sveltejs#2714
  treat requestAnimationFrame as a noop on the server
  site: actions tutorial: destroy is not required (sveltejs#2776)
  Allow binding of <details> open
  site: add /faq redirect to GitHub wiki FAQ
  fix case sensitive import name, improve tsconfig
  type declarations for bundled files
  convert everything to TypeScript
  check for unknown props when creating component
  cleanup, improve comments
  typecheck npm script
  workarond for estree-walker related typings conflict
  compile/render-dom and other remaining typings
  ...

# Conflicts:
#	src/compile/render-dom/wrappers/Element/index.ts
colincasey added a commit to colincasey/svelte that referenced this pull request May 28, 2019
…ings

* master: (66 commits)
  -> v3.4.3
  always add raw property to text nodes
  flesh out in/out transition tutorial chapter (sveltejs#2792)
  code style
  fix test
  fix tests
  Fix CRUD example to allow changing input values.
  Fixes sveltejs#2714
  treat requestAnimationFrame as a noop on the server
  site: actions tutorial: destroy is not required (sveltejs#2776)
  Allow binding of <details> open
  site: add /faq redirect to GitHub wiki FAQ
  fix case sensitive import name, improve tsconfig
  type declarations for bundled files
  convert everything to TypeScript
  check for unknown props when creating component
  cleanup, improve comments
  typecheck npm script
  workarond for estree-walker related typings conflict
  compile/render-dom and other remaining typings
  ...
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.

4 participants