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

Fix Video Scaling Responsiveness #69

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

amalnanavati
Copy link
Contributor

This PR is a fix to an issue that was unable to be resolved in #60 . Specifically, #60 ended up with a bunch of hardcoded scale factors on pages with the robot's video stream, to attempt to make it consistently render properly. However, hardcoded scale factors is not a good code-writing style and is ripe for bugs. This PR addresses that by consistently adding views that take up the full specified width, and then computing the video stream size from those parent components.

Key insights:

  • We should be setting the image’s width and height with respect to the width and height of the parent View, not with respect to the window width and height.
    • Key implementation detail: In order to do that, the parent’s width and height must not adjust based on the content within it. Therefore, the width and height props for the parent element must be set. Furthermore, a reference to the parent must be passed in to the component that is rendering the video.
  • We don’t need to use phantom buttons to fill space if we are intentional about flex box and use width: 100%, height: 100% to have the views themselves fill up the space.
    • Corollary: by being intentional about Views, we can even remove the need for phantom buttons from the footer.

This PR was tested on the browser's version of the iPhone 12/13 Pro Max and the iPad iOs 14.7.1, in both portrait and landscape mode.

@amalnanavati amalnanavati requested a review from Raidakarim June 29, 2023 02:03
@amalnanavati amalnanavati mentioned this pull request Jun 29, 2023
4 tasks
@Raidakarim Raidakarim merged commit 0b4d05f into raidak/responsiveness Jun 30, 2023
@Raidakarim Raidakarim deleted the amaln/fix_video_responsiveness branch June 30, 2023 16:30
amalnanavati added a commit that referenced this pull request Jul 12, 2023
* add to PR template

* add new .md

* add md link

* undo

* Update ResponsivenessTesting.md

* update md file

* initial code for responsiveness

* landscape views

* landscape changes

* add arrow code

* rearrange buttons

* biteSelect

* Update ResponsivenessTesting.md

* Update ResponsivenessTesting.md

* Update ResponsivenessTesting.md

* Update ResponsivenessTesting.md

* custom hook and landscape for video

* review response

* button width and height

* add footer width

* width

* remove copied code in footer

* add comments

* variables of font size and wisth

* change dependency

* comma check done

* biteSelection position fix

* fix variables

* add margin variable

* footer width adjust

* variable in-line

* comments add

* add format

* add video in helper

* callback add

* useEffect for video and plateLocator lock

* food image button fixes

* acqusition button fix

* bite done and seletion

* skip button resize

* skip fix

* skip button space remove

* mask

* mask update

* add

* use percentage of size

* mask updates

* mask working now

* some PR cooments responded

* progress bar and text auto-sizing

* make footer responsiveness and tested in all device

* adjust header sizes in vw vh

* add responsiveness changes

* some PR response

* PR responses

* Implemented vertical centering (#66)

* Implemented vertical centering

* overlap fix

* header fix

* merged amals pr

---------

Co-authored-by: Raida Karim <[email protected]>

* add circle bar size

* matching top bottom

* small fix

* footer fix

* header fix

* add phantom view

* footer fix

* Fixed circle bar size on RobotMotion page (#68)

fixed circle bar size on RobotMotion page

* responsiveness

* add more

* small changes

* Fix Video Scaling Responsiveness (#69)

fixed video scaling responsiveness

* remove phantom view from footer and robotmotion

* helper add

* PR response

* resizing

* fix after test

* add vw

* lock fix

* add comments

* add height fix

* height fix

* use windoe size

* useWindowSize and header fix

* Fix iOS 100vh Bug & Other Responsiveness Bugs (#71)

* 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]>

---------

Co-authored-by: Amal Nanavati <[email protected]>
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