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

Add tutorial #1968

Merged
merged 3 commits into from
Aug 7, 2021
Merged

Add tutorial #1968

merged 3 commits into from
Aug 7, 2021

Conversation

ranile
Copy link
Member

@ranile ranile commented Jul 23, 2021

Description

Fixes #1384

Very inspired by Kotlin React hands-on guide.

This tutorial is based on function components/hooks. I don't know if it's worth using struct components (after #1961), function components will be part of up-coming release anyway.

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code
  • I have added tests

IssueHunt Summary

Referenced issues

This pull request has been submitted to:


@github-actions
Copy link

github-actions bot commented Jul 23, 2021

Visit the preview URL for this PR (updated for commit 5982ca1):

https://yew-rs--pr1968-tutorial-d0mqk3dk.web.app

(expires Sat, 14 Aug 2021 17:36:46 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link
Contributor

@lukechu10 lukechu10 left a comment

Choose a reason for hiding this comment

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

Made a few suggestions here and there. Looks good overall!

website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved

In a real world application, data will basically never be hardcoded but instead will come from an API. Let's fetch our
videos list from external source. For this we will need to add the following crates:
- [`reqwasm`](https://crates.io/crates/reqwasm)
Copy link
Contributor

Choose a reason for hiding this comment

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

(Side note: nothing against reqwasm but it would certainly be nice if we could suggest using a more "official" library. Since you are also a maintainer for gloo, maybe something like reqwasm could be ported over?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I would love to but I'm holding off because I can't publish gloo. I don't have permissions on crates.io

website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Show resolved Hide resolved
website/src/pages/tutorial.md Show resolved Hide resolved
@fultonm
Copy link

fultonm commented Aug 5, 2021

Hi @hamza1311. Thank you for the wonderful tutorial. I submitted a PR to your branch with some fixes and edits I made while following it here: ranile#1

The CORS fix using trunk serve --proxy-backend=https://yew.rs/tutorial feature did not work for me and I still get a CORS error. Am I missing something else?

@ranile
Copy link
Member Author

ranile commented Aug 5, 2021

Hi @hamza1311. Thank you for the wonderful tutorial. I submitted a PR to your branch with some fixes and edits I made while following it here: ranile#1

Thanks for that. Can you please submit that as a PR review instead? It's a lot easier discuss it/pick only certain parts of it that way.

The CORS fix using trunk serve --proxy-backend=https://yew.rs/tutorial feature did not work for me and I still get a CORS error. Am I missing something else?

That's because that resource doesn't yet exist. It will once the PR gets merged

website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Show resolved Hide resolved
website/src/pages/tutorial.md Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved
[here](/next/concepts/function-components/pre-defined-hooks#use_state)

:::note
Struct components act differently. See the documentation to learn about those.
Copy link

Choose a reason for hiding this comment

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

Struct components act differently. See the documentation on [Components](https://yew.rs/concepts/components?) to learn about those.

website/src/pages/tutorial.md Outdated Show resolved Hide resolved
website/src/pages/tutorial.md Show resolved Hide resolved
website/src/pages/tutorial.md Outdated Show resolved Hide resolved
@fultonm
Copy link

fultonm commented Aug 6, 2021

Thanks for that. Can you please submit that as a PR review instead? It's a lot easier discuss it/pick only certain parts of it that way.

Done.

That's because that resource doesn't yet exist. It will once the PR gets merged

I would also expect a 404 error in that case. However I get:

127.0.0.1/:1 Access to fetch at 'https://yew.rs/tutorial/data.json' from origin 'http://127.0.0.1:8080' has been blocked by CORS policy:

If I try requesting an image file that does exist such as https://yew.rs/img/logo.png I get the same error.

@ranile
Copy link
Member Author

ranile commented Aug 7, 2021

@fultonm, I've made the changes, feel free to take a look again.

FYI, there's a suggestion button when you add a review comment provides UI for suggestion and makes incorporating them much easier.

@fultonm
Copy link

fultonm commented Aug 7, 2021

Looks good to me!

Ignore my comments about about CORS, I was still specifying the full path to https://yew.rs/tutorial/data.json instead of relative path. I get a non-CORS error as expected now.

And thanks for the suggestion UI tip I see how that works now.

@siku2 siku2 merged commit 2791fe7 into yewstack:master Aug 7, 2021
```

:::note
We're using `unwrap`s here because it is a demo application. In a real world, In a real world, you want to have
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
We're using `unwrap`s here because it is a demo application. In a real world, In a real world, you want to have
We're using `unwrap`s here because this is a demo application. In a real world app, you should use

Copy link
Member Author

Choose a reason for hiding this comment

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

It's too late to comment here, I guess. If you think this change should be incorporated, you should make new PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh lol true 😄 I was just skimming it over and noticed noticed the typo. I completely forgot that it was already merged.
Sorry about that

Copy link
Member Author

Choose a reason for hiding this comment

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

I also just noticed the typo. It would be great if you could fix it in a PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Done #1990

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create full feature battery-included tutorial for yew
6 participants