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

Make App Responsiveness #60

Merged
merged 85 commits into from
Jul 12, 2023
Merged

Make App Responsiveness #60

merged 85 commits into from
Jul 12, 2023

Conversation

Raidakarim
Copy link
Contributor

@Raidakarim Raidakarim commented May 30, 2023

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

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

  • Added a new .md file with documentation on methodology for testing app responsiveness
  • Added a check to PR for future app feature development
  • Shortened moving screens' waiting text
  • Created landscape version for non-moving screens: biteDone, biteSelection, plateLocator and biteAcquisioncheck
  • Created landscape version of footer view: pause, resume, and back buttons + text
  • Created landscape version of moving screens
  • Shortened header height
  • Arrow equal distance arrangement based on screen size and orientation for plateLocator
  • Added space between back and resume buttons in landscape

UPDATE 1: June 2, 2023

  • Modified BiteSelection and PlateLocator landscape screens to have only two side-by-side views instead of three
  • Put text side-by-side to icon images in footer buttons
  • Video automatic resizing based on screen size and orientation for biteSelection, plateLocator, and liveVideoModal (removed hardcoded values) with useLayoutEffect hook
  • Modified moving screens progress bar and time text to be vertically centered

UPDATE 2: June 3, 2023

  • Use current window inner width to define footer's back and resume buttons' width
  • Comment out all newly added functions
  • Check for copy-pasted code and add helper functions to avoid that

UPDATE 3: June 4, 2023

  • Check and remove comma
  • BiteSelection image, text, button positions fix
  • Footer file clear

UPDATE 4: June 5, 2023

  • move showVideo() in helper
  • callBack use in functions
  • Added plate locator header's lock icon: Plate locator is a unique screen among all the other screens because it can't be classified as a fully moving or fully non-moving screen. Initially, it is a non-moving screen. But, when the user clicks on the arrow buttons the robot is in motion. Finally, when the user clicks on "Done", the robot becomes motionless again. To accurately convey these three situations, the lock icon appears in the initial non-moving and post done-clicked situations. When the user clicks on any of the arrow buttons, there is not lock appearing on the header meaning the robot is not stopped.
  • windowSize variable renamed
  • useEffect for video size update in PlateLocator and BiteSelection
  • maskScaleFactor re-calculate for BiteSelection
  • make skip button position less-changing

UPDATE 5: June 6, 2023

  • Skip button--- made height smaller, width took window width
  • BiteDone text fix

UPDATE 6: June 7, 2023

  • BiteSelection handover to Amal

Explain how this pull request was tested, including but not limited to the below checkmarks.

I tested in ubuntu 22; in Chromium Version 113.0.5672.126 (Official Build) snap (64-bit) with iPhone 14 Plus and iPad 12.9 Pro layout in both of the portrait and landscape orientation ensuring for all app pages. I also tested on Laptop and Desktop views and these results seemed to hold:

  • Landscape views render as desired with modified footer buttons, moving and non-moving screens
  • Header takes less space than before
  • Video adjusted based on screen size and orientation
  • PlateLocator arrows remain same in different screens sizes and orientation
  • Checked for console errors from my code; found none

Tests conducted as per the proposed methodology:

1. iPad Pro 12.9 Chrome Portrait (EC2)
Looks good
2. iPad Pro 12.9 Chrome Landscape (EC2)
Looks good but video loading and progressing slowly in BiteSelection, PlateLocator, and LiveVideoModal
3. Desktop Monitor Chrome (EC2)
Looks good
4. iPhone 14 Plus Chrome Portrait (EC2)
Looks good but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal [waited 2 mins and reloaded the page]
5. iPhone 14 Plus Chromium Landscape (Ubuntu)
Looks good
6. iPhone 14 Plus Firefox Landscape (Ubuntu)
Looks good
7. iPhone 14 Plus Firefox Portrait (Ubuntu)
Looks good
8. iPhone 14 Plus Edge Portrait (EC2)
Looks good but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal [waited 1 min and reloaded the page]
9. iPhone 14 Plus Edge Landscape (EC2)
Looks good but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal [waited 30 sec and reloaded the page]
Screenshot 2023-06-05 at 10 36 02 PM

10. Laptop Full Screen Chromium (Ubuntu)
Looks good
11. Laptop Half Screen Chromium (Ubuntu)
Looks good
12. Laptop Quarter Screen Chromium (Ubuntu)
Looks good
13. Laptop Full Screen Firefox (Ubuntu)
Looks good
14. Laptop Half Screen Firefox (Ubuntu)
Looks good
15. Laptop Quarter Screen Firefox (Ubuntu)
Looks good
16. Laptop Full Screen Edge (EC2)
Looks good but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal [waited 30 sec and reloaded the page]
17. Laptop Half Screen Edge (EC2)
Looks good but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal [waited 30 sec and reloaded the page]
18. Laptop Quarter Screen Edge (EC2)
Looks good but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal [waited 30 sec and reloaded the page]

Extra Tests:

a. iPhone 12 Pro Max (Raida’s iPhone) Edge Portrait & Landscape (EC2)
Things got cropped but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal [waited 1 min and reloaded the page]
IMG-6791
IMG-6793
b. iPhone 12 Pro Max (Raida’s iPhone) Safari Portrait & Landscape (EC2)
Things got cropped but video didn’t load in BiteSelection, PlateLocator, and LiveVideoModal like the above a. [waited 1 min and reloaded the page]
c. iPad Pro 12.9 Safari Portrait (EC2)
Looks good but video loading and progressing slowly in BiteSelection, PlateLocator, and LiveVideoModal

Skipped 5 Tests: (Conducted 18 tests out of the listed 23 tests)

  • Couldn’t do 2 Safari tests for iPhone 14 Plus or a recent iPhone, as couldn’t add custom devices
  • Couldn’t do 3 Safari tests for Laptop, as couldn’t add custom devices

Before creating a pull request

  • Format React code with npm run format
  • [N/A] Format Python code by running python3 -m black . in the top-level of this repository
  • 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 Jun 1, 2023
@Raidakarim Raidakarim requested a review from amalnanavati June 1, 2023 21:21
amalnanavati

This comment was marked as resolved.

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.

See in-line comments below.

Also, friendly reminder that when you address the BiteSelection changes, you should also remove the min-width query from there. We shouldn't be adjusting styles based on width, and as #68 illustrates, there are always clever and simple ways to achieve the desired goal without resorting to a width-based conditional.

feedingwebapp/src/Pages/Footer/Footer.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Footer/Footer.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Footer/Footer.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Header/Header.jsx Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/RobotMotion.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/RobotMotion.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/RobotMotion.jsx Outdated Show resolved Hide resolved
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.

  1. I used Fix Video Scaling Responsiveness #69 to address the TODOs and issues in LiveVideoModal and BiteSelection. Please review that, make sure you understand what is going on and why, and merge it in whenever you are ready.
  2. There are a few small comments here remaining for you to address.
  3. Once you address those, let me know and I'll deploy it on EC2. We should then re-run all tests one final time before merging it in.

feedingwebapp/src/Pages/Footer/Footer.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Header/Header.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Footer/Footer.jsx Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/RobotMotion.jsx Outdated Show resolved Hide resolved
@Raidakarim Raidakarim requested a review from amalnanavati July 1, 2023 00:47
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.

See requested changes. Also see requested changes in Slack.

feedingwebapp/src/Pages/Footer/Footer.jsx Show resolved Hide resolved
feedingwebapp/src/Pages/Header/Header.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/BiteSelection.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/BiteSelection.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/PlateLocator.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/PostMeal.jsx Outdated Show resolved Hide resolved
feedingwebapp/src/Pages/Home/MealStates/PreMeal.jsx Outdated Show resolved Hide resolved
@Raidakarim Raidakarim requested a review from amalnanavati July 9, 2023 00:42
@Raidakarim Raidakarim force-pushed the raidak/responsiveness branch from ee1e630 to a4d3572 Compare July 11, 2023 03:38
* Fixed header/footer height

- Increased header/footer height in landscape
- Made all header elements Nav.Link for consistent height

* Fix RobotMotion

- Remove all overrides of text font size from h1/h2/h3... These are already responsive.
- Remove nested h1's, which is invalid DOM nesting

* Fix RobotMotion Bug

- Changing the screen height on resize introduced a bug where RobotMotion re-calls the action. The changes to Home.jsx address that.
- Implements Raida's idea for a simple solution to the iOS '100vh' bug that uses existing helpers

* Fix VideoFeed resizing bug

* Fixed CircleProgressBar size in landscape

* Address iOS orientation change bug

* Remove unused package Div100vh

* Lengthened the delay for window size due to iOS orientation change issue

* Platelocator button height and live video font size increase and robotmotion fontsize modify

* Fix footer font size

---------

Co-authored-by: Raida Karim <[email protected]>
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.

Approved based on review and testing of #71 , which is now the latest commit on this branch

@amalnanavati amalnanavati merged commit de0e5f3 into main Jul 12, 2023
@amalnanavati amalnanavati deleted the raidak/responsiveness branch July 12, 2023 15:45
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.

Make the App Fully Responsive
2 participants