Skip to content

Controls to Navigate Between Meal States #30

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

Merged
merged 31 commits into from
May 9, 2023

Conversation

Raidakarim
Copy link
Contributor

@Raidakarim Raidakarim commented Apr 26, 2023

[Describe this pull request. Link to relevant GitHub issues, if any.]

This PR includes issue #10 deliverables as detailed in the following:

  • Icons design, implementation, and adding text above icons
  • Revamping footer by removing the pause modal and redesigning the pause button which has a onclick triggering implementation of two new side-by-side back and resume buttons
  • A dictionary that is used to contain meal state and corresponding icon image pairings to populate the footer's back and resume button
  • Consistent color scheme implemented for icon buttons the main app screens and the footer
  • Some TODO comments for implementing the previous meal state logic for footer's back button with ideal previous meal state and corresponding icon image

[Explain how this pull request was tested, including but not limited to the below checkmarks.]
I tested the implementations by running the app screens in my local Safari browser and ensuring:

  • the icons appear as desired and goes to desired next action when clicked
  • the footer and its buttons of pause, back and resume button appear as desired; the buttons do desired actions when clicked except the back button
  • the color scheme is easy to follow and the buttons are big enough as suggested by Tyler
  • the dictionary key-value images load in the footer but still need work in the back button

Before creating a pull request

  • Format code with npm run format
  • Thoroughly test your code's functionality, including unintended uses.
  • Thoroughly test your code's responsiveness by rendering it on different devices, browsers, etc.
  • [N/A] Consider the user flow between states that this feature introduces, consider different situations that might occur for the user, and ensure that there is no way for the user to get stuck in a loop.

Before merging a pull request

  • Squash all your commits into one (or Squash and Merge)

@Raidakarim Raidakarim self-assigned this Apr 26, 2023
@Raidakarim Raidakarim requested a review from amalnanavati April 26, 2023 20:18
amalnanavati

This comment was marked as outdated.

@Raidakarim Raidakarim requested a review from amalnanavati May 1, 2023 00:21
amalnanavati

This comment was marked as outdated.

@Raidakarim Raidakarim requested a review from amalnanavati May 1, 2023 21:19
@Raidakarim

This comment was marked as outdated.

amalnanavati

This comment was marked as outdated.

@amalnanavati

This comment was marked as outdated.

@Raidakarim Raidakarim requested a review from amalnanavati May 8, 2023 22:37
@Raidakarim
Copy link
Contributor Author

Here are some additional details regarding this PR's deliverables:
(1) Icons design, implementation, and adding text above icons: I fine-tuned icons by recreating, resizing and re-downloading their svg images in inkscape. There are 5 icons currently: moveAbovePlate, moveToStaging, moveToMouth, moveToBiteAcquisition, and for the pause button. For the moveToBiteAcquisition Icon, I liked and used the icon where it shows bite acquisition in action like the fork being used to lift food from plate. But if the food is on a plate with a fork above it with no motion, it would be similar to what the moveAbovePlate icon does. In fact, I modified the moveAbovePlate icon to have food on plate, as there does not seem to be a possibility when the user would want to move above plate when there is no food on plate (e.g., they are done eating). I made the buttons bigger than before supporting Tyler’s feedback to make it easier to see for people with vision problems. I maintained the same button size for all icon buttons for consistency, and used medium size margins that look good and clear. Also, Tyler mentioned preferring both icons and text rather than only icons or only text in buttons. Thus, I provided accompanied texts above these icon buttons to make it clear to the user for what purpose the below icon button is used for. The responsiveness aspects of the icons will be tested in app responsiveness issue #9.
(2) Footer-- Pause, Resume and Back buttons: Following Tyler’s advice, I redesigned the pause icon button to be bigger and red, easily indicating the emergency cancellation option for current robot motion. I removed the previous pause modal. If the pause button is clicked, then the back and resume buttons show up in the footer. The back and resume buttons’ text and image should get updated based on previous and current meal state the robot is in, respectively. Currently this works only for the resume button and the TODO comments indicate the places the back button logic should go. The back button click should also change the robot’s state to previous meal state from current meal state. Clicking on either back and resume buttons brings back the pause button on the footer.
(3) Dictionary Logic: Unlike the icon buttons used in the specific meal states .jsx files that are guaranteed to appear in that particular meal state, the footer back and resume buttons appear in all meal states where the robot moves. Therefore, the icons used in back and resume buttons are not fixed and are determined based on the robot’s previous and current meal states. The previous state logic is still being figured out. I created a dictionary in the Constants.js file that stores the 4 moving robot states (moveAbovePlate, moveToStaging, moveToMouth, moveToBiteAcquisition) as keys and their icon images as values. For the moveToStaging icon the footer icon image is saved differently with “_footer.svg” at the end. This is because this specific icon svg image is saved with different sizes to respectively fit the main icon button and footer icon buttons. From the footer file I accessed and used the dictionary image values associated with current meal state for the resume button. All icon images are saved in the robot_state_imgs folder of public.
(4) Color Scheme: Inspired by traffic signals for colors indicating the nature of control for robot motion, I used the following color scheme throughout the robot motion control icon buttons in the main app screens and the footer icon buttons--
Red: accident / emergency / pause / stop / halt
Yellow: unintended situation / warning / return / user changes mind
Green: success / smooth progression

Copy link
Contributor

@amalnanavati amalnanavati left a comment

Choose a reason for hiding this comment

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

Great work! It is quite close to being done :) I left a few small comments in-line, and have a few more here:

  1. I realized that we should never have the robot move back to BiteAcquisition without first going to BiteSelection. Because say the robot attempted a bite acquisition and failed. In the process, it may have moved the food around on the plate. Therefore, the old mask the user specified for the food item is no longer valid, and the user has to re-select a bite to get a new mask given the new locations of the foods. See Issue Should we add a "Reselect Bite" option to BiteAcquisitionCheck? #39 for additional discussion about this.
    • As a result of this, the button in BiteAcquisitionCheck to try again should instead go to "MovingAbovePlate" -- could you change that state transition and icon?
    • Once you do this, you'll notice that move_to_bite_acquisition.svg is no longer used. So you can delete if you want. If you decide not to delete it (just in case we decide to use it in the future), you should make the changes to it I mentioned in the in-line comments.
  2. You'll notice that the branch is out-of-date with main (see the bottom of this PR). Before you next mark it as ready for re-review, could you "Update Branch"? After you do that on the web, you'll have to git pull locally to get that change.
  3. I created Issue Implement "back" button functionality after the user has clicked pause #40 for the "Back" button functionality. That can be focused on after this PR is merged in.

@Raidakarim Raidakarim requested a review from amalnanavati May 9, 2023 05:57
Copy link
Contributor

@amalnanavati amalnanavati left a comment

Choose a reason for hiding this comment

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

Good work, this all looks good to me.

There is one thing I forgot in my last review: adding an Acknowledgements.md file with the icons we used. As I had mentioned when we were first creating the icons, it is important to keep track of the icons we use in order to acknowledge them. Can you add that file, and add follow the instructions here or here to acknowledge all the icons we used?

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.

2 participants