New frontend web client written using Rust and Yew.rs #459
Conversation
There was a problem hiding this comment.
Thanks! I didn't review everything yet because this is quite big. Also I have a high-level concern which is that Yew seems to force putting the state of a component in its parent through the notion of properties. I find this quite weird, but maybe that's how it's supposed to be? It seems that implementing Component manually would fix this because we have control over the Message associated type. But then, maybe this will make other things complicated. I would be curious to know if you tried implementing Component explicitly, such that each component has its own state and the properties are only used for configuration by the parent, while the state is modified through message passing. This seems to better match the fact that the app is driven by a websocket which is about message passing.
crates/runner-host/crates/web-client/components/button_pressed.svg
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/hooks/use_runner_connection.rs
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/hooks/use_runner_connection.rs
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/hooks/use_runner_connection.rs
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/hooks/use_runner_connection.rs
Outdated
Show resolved
Hide resolved
ia0
added
for:maintainability
|
I guess you are working incrementally or did not push your changes. Feel free to click the "Re-request review" button once the PR is ready for review. Thanks! |
Will do. I think the confusion is I seemed to have messed something up and lost some changes that I made and I hadn't noticed. Something with committing with the web UI and then making and pushing local changes resulted in me losing some work. I think I have redone all of my changes now. I'm going to do another pass and then I'll request a review again. Sorry about that. |
…ckets. For the moment just sends a fake board ready event so server will send messages as the board is not yet implemented.
Connected board to websocket connection and added button presses and led light up Connected board to websocket connection Also added button presses and led light up Connected board to websocket connection Also added button presses and led light up Connected board to websocket connection Also added button presses and led light up
And removed copyright notice
Changed to match statement Fixed copyright notice Fixed gitignore to ignore dist dir as well Changed log to use inline style Changed from use_state_eq to use_state Fixed use effect closures to be more idomatic rust. Removed unused imports Moved static assets to data directory Added test.sh Incorprated clippy suggestions Reorded fields Co-authored-by: Julien Cretin <github@ia0.eu> Reduced nesting in light conditional Reduced scopes in closure Co-authored-by: Julien Cretin <github@ia0.eu> Reordered use and mod Co-authored-by: Julien Cretin <github@ia0.eu> Remove scope and put to clone inline Co-authored-by: Julien Cretin <github@ia0.eu> Fixed HTML formatting Fixed html formatting
chris-dietz
force-pushed
the
web-client-rs
branch
3 times, most recently
from
d395dd7
to
3cb5c01
Compare
Co-authored-by: Julien Cretin <github@ia0.eu>
Co-authored-by: Julien Cretin <github@ia0.eu>
Changed to self closing styled elements
chris-dietz
force-pushed
the
web-client-rs
branch
from
3cb5c01
to
a0f7c26
Compare
|
Alright I think I have incorporated all the feedback and I also went back and cleaned up the commit history. So I think it should be ready to be re-reviewed now. Sorry for any confusion. I'll do a better job of waiting to push until I'm ready for things to be reviewed in the future. |
There was a problem hiding this comment.
Thanks! I'll apply the suggestions and merge. Then I'll merge main into dev/web-ui, so you'll have to pull dev/web-ui and work on that branch. In particular, now that we have a branch for this work, you can remove the automatic opening of the old UI and do any change convenient to you until we can replace the old UI, at which point we can merge this branch into main.
crates/runner-host/crates/web-client/src/board_components/button.rs
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/board_components/button.rs
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/board_components/button.rs
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/board_components/led.rs
Outdated
Show resolved
Hide resolved
crates/runner-host/crates/web-client/src/board_components/led.rs
Outdated
Show resolved
Hide resolved
I spoke too fast. Looks like this PR doesn't have "allow edits by maintainers", so I can't. I'll wait until you apply the suggestions then. |
ia0
removed
for:maintainability
I applied the suggested changes. So should be good to merge I think. :) |
Co-authored-by: Julien Cretin <github@ia0.eu>
chris-dietz
force-pushed
the
web-client-rs
branch
from
f2f71f1
to
1ade202
Compare
|
I've merge main into dev/web-ui. You'll need to continue working from that branch. |
Adds a new web client runner built from the ground up using rust and yew.rs.
Part of #261
Right now web client must be manually built and launched separate to launching the backend. A followup PR will add a new flag to tell runner to use the new web client. Both the old and new client will be supported until the new front end has been thoroughly tested and has feature parity.