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

Implemented vertical centering #66

Merged
merged 5 commits into from
Jun 11, 2023

Conversation

amalnanavati
Copy link
Contributor

@amalnanavati amalnanavati commented Jun 10, 2023

Implements vertical centering on top of #60 . There were a few key insights to this PR:

  • In order for flex Views to work throughout the app, all parent elements have to also be flex Views. Hence, this PR ensures all parent elements are Views from index.js down.
    • This (mostly) worked for all pages but RobotMotion, which had the issue that the footer was fixed, and therefore had absolute position instead of relative position. I made it have relative position but always appear on the bottom, so that it would still work within a flex View.
  • By default, div and View will fit to the smallest space possible in order to fit its content. To avoid that, we need to specify a width or height.
  • We have to be intentional about 100vw/100vh vs. 100%. The former are with regards to the whole screen, the latter is with regards to the parent component.
  • justifyContent: 'center' is with respect to the parent element. Inspecting element is a great way to identify the width/height of the parent element and determine how to adjust it accordingly.
  • The app had multiple unnecessary <> and <React.Fragment> tags. These are only necessary if the returned HTML code consists of multiple sibling components, and even then only one of the above is necessary. React requires all returned HTML code to be within a single tag.
  • This app has a strange mix of div and View when, in reality, they are equivalent but in different frameworks (React vs. React Native Web). Each framework works best with other components from the framework, so we should entirely use one or the other. Even flex in React Native Web is equivalent to Flexbox in CSS, with slightly different names. I created Issue Move to a Single Framework + Web App Revamp For Single-Screen Pages #67 for that.

{withoutAcquireButton()}
{debugOptions()}
</View>
<View style={{ flex: 'auto', flexDirection: dimension, justifyContent: 'center', alignItems: 'center', width: '100%' }}>
Copy link
Contributor

@Raidakarim Raidakarim Jun 10, 2023

Choose a reason for hiding this comment

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

Thanks for working on it. I really appreciate it. I tested this PR on Firefox and Chromium browsers in my Ubuntu VM in all 4 device layouts. Only BiteSelection page seems to have a problem in iPad's and laptop's Portrait orientation. PLease see the attached images below. These problems do not arise in iPhone and desktop monitor layouts. The "Locate Plate" and "Done Eating" buttons seem to overlap with the view. I tried adding a little bit margin below those two buttons, but the problem still exists. What do you think? I can also think more about it later after my PR is somewhat ready.
Screenshot 2023-06-10 at 7 53 19 AM
Screenshot 2023-06-10 at 7 44 22 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Once this problem is solved, I will review this PR again looking through the code changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for finding this. This is because locate plate and done eating is in a div (which is part of React) and not a view (which is part of React Native). These frameworks work best if we use only one of them, but this app uses a mix of both.

I’ll let you fix this bug by changing the div to a view and maybe adding some style components to the view.

@Raidakarim Raidakarim merged commit 2b2b696 into raidak/responsiveness Jun 11, 2023
@Raidakarim Raidakarim deleted the amaln/vertically_center branch June 11, 2023 15:04
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