Skip to content

Add Function Components Example#2088

Merged
mc1098 merged 6 commits into
yewstack:masterfrom
yoroshikun:function-components-example
Oct 16, 2021
Merged

Add Function Components Example#2088
mc1098 merged 6 commits into
yewstack:masterfrom
yoroshikun:function-components-example

Conversation

@yoroshikun
Copy link
Copy Markdown
Contributor

@yoroshikun yoroshikun commented Sep 26, 2021

Description

Simple ToDo Web App using function components and hooks

Fixes #2072

Checklist

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

TODO Before ready

  • Split components into own function components (Input, todo item)
  • Split logic into separate file
  • Add filtering (use_state and use_effect use case example?)
  • Add persistence (gloo_storage)
  • Removal of redundant code
  • Review use of feather icons
  • Final code cleanup

@mc1098 mc1098 added the A-examples Area: The examples label Sep 26, 2021
@mc1098
Copy link
Copy Markdown
Contributor

mc1098 commented Sep 26, 2021

🙏 I'm sorry; I think the TodoMVC https://todomvc.com/ has a standard style to it - also I do want the todo examples to be 1 to 1 with the only difference being the struct style approach and the function component approach.

I did just start up your version and I do quite like the style, always more of a dark theme person myself :)

@yoroshikun yoroshikun force-pushed the function-components-example branch 2 times, most recently from 407b6d3 to 0863616 Compare September 30, 2021 13:08
mc1098
mc1098 previously requested changes Oct 1, 2021
Copy link
Copy Markdown
Contributor

@mc1098 mc1098 left a comment

Choose a reason for hiding this comment

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

A few changes suggested but its mostly polish and performance tips - the todo application works well 🎉 I like how its been broken up into smaller components and was nice to see the "pure" footer component.

I'm seeing a pattern with cloning the output of hooks here and I just want to touch upon it. So often when we are cloning something like the UseStateHandle we do so because we are sending it to a Callback that must live for some unknown period of time and therefore we want it to be 'static. There are times, like getting the completed count where you can borrow the UseReducerHandle (state variable in this case) for a known and short period of time to build up some separate result and in these cases you don't have to clone the handle - you borrow and use it and then give it back for the following code to use.

Comment thread examples/function_todomvc/src/components/entry.rs Outdated
Comment thread examples/function_todomvc/src/components/entry.rs Outdated
Comment thread examples/function_todomvc/src/components/entry.rs Outdated
Comment thread examples/function_todomvc/src/components/entry.rs Outdated
Comment thread examples/function_todomvc/src/components/entry.rs Outdated
Comment thread examples/function_todomvc/src/main.rs Outdated
Comment thread examples/function_todomvc/src/main.rs Outdated
Comment thread examples/function_todomvc/src/main.rs Outdated
Comment thread examples/function_todomvc/src/main.rs
Comment thread examples/function_todomvc/src/main.rs
@mergify mergify Bot dismissed mc1098’s stale review October 1, 2021 05:04

Pull request has been modified.

@yoroshikun yoroshikun marked this pull request as ready for review October 1, 2021 07:26
@yoroshikun yoroshikun force-pushed the function-components-example branch from f1a4bf6 to ac429d1 Compare October 2, 2021 07:46
@yoroshikun yoroshikun requested a review from mc1098 October 2, 2021 07:46
@siku2 siku2 added the hacktoberfest-accepted See https://hacktoberfest.digitalocean.com label Oct 2, 2021
@yoroshikun yoroshikun force-pushed the function-components-example branch from ac429d1 to a46cbab Compare October 2, 2021 14:22
mc1098
mc1098 previously requested changes Oct 2, 2021
Copy link
Copy Markdown
Contributor

@mc1098 mc1098 left a comment

Choose a reason for hiding this comment

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

A few more suggestions some of which are optional or just pointing out certain things :)
So nearly there and sorry it's been taking me a bit longer to review these changes because I'm playing with things and tweaking impls and thinking about whether there are changes yew could make to make this all smoother.

Comment thread examples/function_todomvc/src/components/entry.rs Outdated
Comment thread examples/function_todomvc/src/main.rs Outdated
Comment thread examples/function_todomvc/src/main.rs Outdated
Comment thread examples/function_todomvc/src/components/input.rs Outdated
@mergify mergify Bot dismissed mc1098’s stale review October 7, 2021 12:01

Pull request has been modified.

@yoroshikun yoroshikun force-pushed the function-components-example branch 2 times, most recently from fb765c1 to f3222e6 Compare October 7, 2021 13:01
@yoroshikun yoroshikun force-pushed the function-components-example branch from f3222e6 to 17b92fa Compare October 8, 2021 15:44
mc1098
mc1098 previously requested changes Oct 9, 2021
Copy link
Copy Markdown
Contributor

@mc1098 mc1098 left a comment

Choose a reason for hiding this comment

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

Mostly minor touches now.
I want a comment to explain that custom hook, otherwise we are going to have to keep explaining it in Discord for new yew users. The custom hook adds a bit of complexity but IMO is worth it, as it demonstrates implementing a raw custom hook which is not trivial :)

Not sure if you've skimmed over this at all @siku2 but I think this is the final round of review before I give it the green light - do you have anything to add to this review?

Comment thread examples/function_todomvc/src/components/entry.rs
Comment thread examples/function_todomvc/src/hooks/use_bool_toggle.rs Outdated
Comment thread examples/function_todomvc/src/hooks/use_bool_toggle.rs Outdated
Copy link
Copy Markdown
Member

@siku2 siku2 left a comment

Choose a reason for hiding this comment

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

@mc1098 yeah I think this is pretty much good to go.
One thing that appears to be missing is the entry in the examples table in examples/README.md.

Comment thread Cargo.toml
Comment on lines 16 to 18
"examples/dyn_create_destroy_apps",
"examples/function_todomvc",
"examples/file_upload",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Funky formatting?

@mergify mergify Bot dismissed mc1098’s stale review October 12, 2021 10:38

Pull request has been modified.

@yoroshikun yoroshikun force-pushed the function-components-example branch from c02c835 to 24e6747 Compare October 12, 2021 10:39
Copy link
Copy Markdown
Contributor

@mc1098 mc1098 left a comment

Choose a reason for hiding this comment

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

Expanding a bit on that comment but thats it.

Comment thread examples/function_todomvc/src/components/entry.rs Outdated
@mc1098 mc1098 merged commit 05728e1 into yewstack:master Oct 16, 2021
@mc1098
Copy link
Copy Markdown
Contributor

mc1098 commented Oct 16, 2021

We made it 🎉

Sorry for being so picky at points, part of it was some exploring of the hooks and function component API on my part too :)
Any feedback on function components or hooks in general then ofc feel free to either mention it here or create an issue going into what you felt worked and didn't work and what was clumsy. :)

Thank you for all the hard work ❤️

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

Labels

A-examples Area: The examples hacktoberfest-accepted See https://hacktoberfest.digitalocean.com

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Example using function components

3 participants