2023 Web App Revamp #5
Conversation
- Began with code from the `2023PreDeployment` branch - Split each component into a separate file - Added thorough in-code documentation - Adding eslint and prettier checks for readability and consistency - Implemented intentional splits between local and global state
amalnanavati
force-pushed
the
amaln/web_app_revamp
branch
from
edd39a7
to
c8d8545
Compare
|
@taylorkf The code for this PR is ready to review. My only remaining TODO before it is fully ready to review is updating the readme, but since it's a lot of code I figured you could get started while I'm working on the readme. Please be brutal in your review; I want to make this code easy and logical for newcomers to onboard onto, and I want it to be easy for multiple people to work on this codebase at once (e.g., unlikely to need to modify the same files too much). As opposed to reading the files in the alphabetical order github presents them in, I'd recommend starting from |
|
Added contribution guidelines and updated Tech Documentation. Fully ready to review |
There was a problem hiding this comment.
@amalnanavati I love how much more organized this is!!
And, please let me know if any of my comments don't make sense! I am happy to explain!
| case MEAL_STATE.R_StowingArm: | ||
| return <StowingArm debug={debug} /> | ||
| case MEAL_STATE.U_PostMeal: | ||
| return <PostMeal debug={debug} /> |
There was a problem hiding this comment.
Did we end up deciding that the app would just say "bite done" when moving away from the mouth and back to above the plate, or should we add another state for slowly backing up from the mouth?
There was a problem hiding this comment.
From the user perspective, I think they should just click a "Bite Done" button. Whether that is implemented as a "slowly back away" or "go straight above plate" or something else should be left to the ROS action that is called when that button is clicked, imo.
| /** | ||
| * Callback function for if the user decides to cancel the bite. | ||
| */ | ||
| function cancelBite() { |
There was a problem hiding this comment.
Should this button take the user back to the food selection screen? Right now in debug = true, pressing "cancel bite" does nothing visually, and you still have to advance forward through the bite acquisition to move on.
There was a problem hiding this comment.
Hmm, are you sure "Cancel Bite" did nothing? It should go back to the "Moving Above Plate" screen+state, which it does on my computer and is what I see reflected in the code.
I think reverting to "Moving Above Plate" makes more sense than food selection, because if you cancel when the fork is very close to the plate, it will be too close to make a food selection. Food Selection will come once moving above plate is done.
In service of #16
Although the 2023 web app we've been working on (
2023PreDeploymentbranch) has a functioning UI, it has several process issues:This is not anyone's fault, because when we were initially developing the web app we saw it as a small component of the overall feeding system. But the web app is will now be the central hub that users will use to interact with the system. Hence, it is crucial to have high quality code for the web app, and a process to ensure the code stays high quality.
This PR is the first step towards that, by taking the code in the
2023PreDeploymentbranch, breaking it up into bite-sized, logically contained functions, adding thorough in-code documentation, and running it through a linter and formatter to ensure (mostly) consistent style.Before creating a pull request
npm run formatBefore merging a pull request
Squash and Merge)