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

Fix broken examples (#1289) - batch #1 #1340

Merged
merged 15 commits into from
Jun 26, 2020

Conversation

numerodix
Copy link
Contributor

@numerodix numerodix commented Jun 21, 2020

Description

The first batch of small fixes/improvements to the examples. (See the commit messages for rationale on each individual change.)

At this stage I'm not changing how the static directory is used and I'm only updating examples that work with the shared static.

Given that some examples have/need a custom static I'm thinking that build.sh might check inside each example folder to see if there is a static and if so - use that instead. It could then print instructions on how to run the example (given that it can't be run out of the shared static). This would still allow most examples to share a common static.

`python3 -m http.server --directory` flag was added in Python 3.7.
Change instructions to cd into static and skip the flag instead -
makes it work with earlier Pythons too.
The minimal example has a button, but clicking it does nothing. This
makes it hard to tell whether the example works or not.

This adds a label that is update when the button is clicked.
* Add a heading to each scene to make it more obvious what the page is
for.

* Improve layout of input form by stack the inputs vertically.

* Add a little space between entries when displaying the list.

* Add a hint to the Description input that Markdown can be used.
* Add a second button that demonstrates an unsuccessful fetch.

* Render the markdown document (since we're fetching markdown after
all).

* Remove vague comment that's not really helping.
* Include a textual explanation of what the example is doing.
examples/futures_wp/src/markdown.rs Outdated Show resolved Hide resolved
examples/inner_html/src/lib.rs Outdated Show resolved Hide resolved
@numerodix
Copy link
Contributor Author

Is there anything else you'd like me to change, @teymour-aldridge ?

Copy link
Contributor

@teymour-aldridge teymour-aldridge left a comment

Choose a reason for hiding this comment

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

Other than these changes, LGTM

examples/README.md Show resolved Hide resolved
examples/crm/src/lib.rs Outdated Show resolved Hide resolved
examples/inner_html/src/lib.rs Outdated Show resolved Hide resolved
@numerodix
Copy link
Contributor Author

numerodix commented Jun 25, 2020

If we're happy with this one let's merge it and I'll do another batch of fixes.

Looks like the travis build stalled. The jobs themselves ran to completion but the overview thinks they're still in progress?

@teymour-aldridge
Copy link
Contributor

@numerodix Not sure what's up ith the Travis build but it looks like two were created and only one has suceeded.
Only @jstarry can merge things; they also have a job (considering which, it's amazing the amount of time + effort they put into Yew) 😄
I guess it will be merged at some point soon, if there are no objections from him.
You could open another PR if needed, probably avoiding the files you've modified in this PR (for fewer merge conflicts).

Copy link
Member

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the clean up @numerodix! And thanks @teymour-aldridge for reviewing this while I was busy 👍

Just one comment 😄

examples/inner_html/src/lib.rs Outdated Show resolved Hide resolved
@teymour-aldridge
Copy link
Contributor

teymour-aldridge commented Jun 25, 2020

And thanks @teymour-aldridge for reviewing this while I was busy 👍

No problem; you do an awesome job maintaining Yew and I'm happy to try and help where I can!

include_str!() is a neat solution 😄

@mergify mergify bot dismissed jstarry’s stale review June 26, 2020 11:41

Pull request has been modified.

@jstarry jstarry merged commit 7ce0bd8 into yewstack:master Jun 26, 2020
@jstarry
Copy link
Member

jstarry commented Jun 26, 2020

Really appreciate the fixes @numerodix!

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