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 info to website #3403

Closed
wants to merge 4 commits into from
Closed

Conversation

moyeah
Copy link
Contributor

@moyeah moyeah commented Sep 18, 2023

Description

Add more info to website.

Remove whitespace in tutorial.
Changes to be committed:
	modified:   website/docs/tutorial/index.mdx
	modified:   website/versioned_docs/version-0.20/getting-started/build-a-sample-app.mdx
	modified:   website/versioned_docs/version-0.20/tutorial/index.mdx
@github-actions
Copy link

github-actions bot commented Sep 18, 2023

Visit the preview URL for this PR (updated for commit 75d3cab):

https://yew-rs--pr3403-add-info-to-website-7u84pp0u.web.app

(expires Sat, 30 Sep 2023 12:20:52 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

github-actions[bot]
github-actions bot previously approved these changes Sep 23, 2023
Copy link
Member

@ranile ranile left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I merged master into your branch. There's a few comments here to be addressed though

@@ -46,7 +46,7 @@ To convert this simple command line application to a basic Yew web application,

#### Update Cargo.toml

Add `yew` to the list of dependencies.
Add `yew` to the list of dependencies editing file:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Add `yew` to the list of dependencies editing file:
Add `yew` to the list of dependencies by updating your Cargo.toml:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hamza1311 suggested changes implemented in PR #3406.

```toml title="Trunk.toml"
[serve]
# The address to serve on.
address = "127.0.0.1" // "0.0.0.0" for WAN
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
address = "127.0.0.1" // "0.0.0.0" for WAN
address = "127.0.0.1"

WAN is incorrect. 0.0.0.0 makes the port exposed on the local network. It has to be exposed. Also, it not recommended to use trunk for anything else other development so I would like to not recommend this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hamza1311 "0.0.0.0 for WAN" is for remote development (ssh client). I agree that is not recommended! But any alternative for this cases?

application if you modify any source files.
This will fail if the socket is being used by another application.
By default server will listening at address '127.0.0.1' and port '8080' [localhost:8080](http://127.0.0.1:8080).
To change it, create the following file and edit as needed:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To change it, create the following file and edit as needed:
To change it, create the [Trunk config file](https://trunkrs.dev/configuration/#trunk-toml) and edit as needed:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hamza1311 suggested changes implemented in PR #3406.

@ranile ranile dismissed github-actions[bot]’s stale review September 23, 2023 12:14

Unintended automated review

@ranile
Copy link
Member

ranile commented Sep 23, 2023

These changes were included already so I'm going to close this PR

@ranile ranile closed this Sep 23, 2023
@moyeah moyeah deleted the add-info-to-website branch September 23, 2023 13:55
@moyeah moyeah mentioned this pull request Sep 23, 2023
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.

2 participants