seo footer flex testing #15420

grgia · 2023-02-23T19:16:07Z

Details

Fixed Issues

$ GH_LINK
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

Verify that no errors appear in the JS console

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

I linked the correct issue in the ### Fixed Issues section above
I wrote clear testing steps that cover the changes made in this PR
- I added steps for local testing in the Tests section
- I added steps for the expected offline behavior in the Offline steps section
- I added steps for Staging and/or Production testing in the QA steps section
- I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
- I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
I included screenshots or videos for tests on all platforms
I ran the tests on all platforms & verified they passed on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
I followed proper code patterns (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  - If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I followed the guidelines as stated in the Review Guidelines
I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
I verified that if a function's arguments changed that all usages have also been updated correctly
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

grgia · 2023-02-23T19:18:25Z

Matches behavior to staging on IOS and Safari:

Screen.Recording.2023-02-23.at.11.16.21.AM.mov

Screen.Recording.2023-02-23.at.11.16.35.AM.mov

But broken on android and chrome:

(should push up terms)

Screen.Recording.2023-02-23.at.11.17.42.AM.mov

(causes problems when scroll down and click)

Screen.Recording.2023-02-23.at.11.17.16.AM.mov

grgia · 2023-02-23T20:20:06Z

The behavior on chrome should be the behavior on android and vice versa...

Referencing this recent PR:
#15246

grgia · 2023-02-25T00:21:31Z

I cannot figure out what to do about this bug on mobile chrome

Screen.Recording.2023-02-24.at.4.20.15.PM.mov

cc @roryabraham

… not web.

grgia · 2023-02-25T09:17:55Z

I think I finally figured things out- I have a write up for it, but I'm gonna pass out so will copy changes to main PR / post my update in the morning

tgolen · 2023-02-25T17:48:52Z

src/pages/signin/SignInPageLayout/Footer.js

+                            )
+                            : (


Suggested change

)

: (

) : (

tgolen · 2023-02-25T17:49:44Z

src/pages/signin/SignInPageLayout/index.js

+                )
+                : (


Suggested change

)

: (

) : (

src/pages/signin/SignInPageLayout/signInPageStyles/index.native.js

tgolen · 2023-02-25T17:51:26Z

src/pages/signin/Socials.js

+                key={social.link}
+            >
+                {({hovered}) => (
+                    <Icon src={social.iconURL} height={variables.iconSizeLarge} width={variables.iconSizeLarge} fill={hovered ? themeColors.link : themeColors.textLight} />


Please move all the attributes to their own lines since there are so many of them.

tgolen · 2023-02-25T17:52:13Z

src/styles/colors.js

@@ -34,6 +34,7 @@ export default {
    greenDefaultButtonPressed: '#467164',
    greenDefaultButtonDisabled: '#8BA69E',
    midnight: '#002140',
+    green700: '#23503B',


What does the 700 mean?

This is the key for the color in figma

tgolen · 2023-02-25T17:53:05Z

src/styles/styles.js

+        fontSize: variables.fontSizeNormal,
+        paddingTop: 64,
+        paddingHorizontal: 32,
+        maxWidth: 1100,


Could you please add a comment for why 1100 is the specific value? Does it relate to an existing breakpoint?

grgia · 2023-02-27T21:47:00Z

Before I forget, here's a follow up summary for the keyboard bug I encountered:

For android chrome there were a couple things happening here:

(On Mobile Chrome only) the window is resized when the keyboard is opened, which causes the sign-in form to shrink (unwanted behavior). This is because props.windowHeight is returning height - keyboard height.
ScrollView has a feature where it autoscrolls to TextInput. However, this autoscroll was acting buggy. There are some open issues I found similar to this from the last year, so perhaps this was a library issue.

facebook/react-native#33164
software-mansion/react-native-screens#1447

Because of this, setting keyboardDismissMode='on-drag' to dismiss the keyboard when the user scrolls down to the footer caused the keyboard to flash/not open because (my theory) the auto-scroll to TextInput was being recorded as a drag. If we wanted to prevent the user from scrolling with the keyboard open, we might need to look more into this theory.

cc @roryabraham incase you're interested :)

grgia · 2023-02-28T17:34:41Z

Going to close out this test branch for now :) Moved changes to main branch here

grgia added 30 commits January 30, 2023 10:14

Add skeleton for component

00927cd

Merge branch 'main' into georgia-SEO-footer

6793a55

Temporary Block

27d325c

Merge branch 'main' into georgia-SEO-footer

d444d96

Merge branch 'main' into georgia-SEO-footer

e7d634a

Add footer columns/row titles into en/es.js

48278e5

Add data and basic structure of footer component / styles

c3f3b46

Add flex container to footer

1b5c2b3

Add SEO wordmark SVGsk

c437e27

Merge branch 'main' into georgia-SEO-footer

f4081cf

Initial styling

112e5b1

Add footer scrolling (below fold) on small / large screen

dcd56e4

Merge branch 'main' into georgia-SEO-footer

ab7661f

Add colors

225a093

Move license / add copywrite

a82f0ad

Merge branch 'main' into georgia-SEO-footer

d5e8ac8

Scrollview working on all platforms

1743e36

Styling Footer on Mobile and Web

606d41d

Add socials links, more style tweakss

f4f498c

Add social icons, use dynamic date for copyright, modify color names

a3545fb

Use updated social svgs

2b89ce8

Merge branch 'main' into georgia-SEO-footer

1e5abcf

Fix bug from merge- localizer should have transparent BG

6daa052

Fix Terms and License paragraph spacing

4ef06f6

Clean up styles, use 2 columns when medium screen size

0288cde

Merge branch 'main' into georgia-SEO-footer

028eaeb

Ensure that the signin graphic is always the same height as the screen

9ef6e09

Add initial spanish translations (needs review)

7dd193c

Spanish Fixes

b8e1666

Merge branch 'main' into georgia-SEO-footer

43c923e

grgia added 14 commits February 17, 2023 10:14

Use Dot URL in const, remove extra lines/comments

f1f5ca2

Rename TermsAndLicenses to Licenses

da20776

Merge branch 'main' into georgia-SEO-footer

636caba

Use insets when calculating height of signInContent

12c947e

Move constants into CONST/Variables

4f9a32c

Revert removing outside view in Footer

3105818

Simplify bottom terms style

a8af083

Merge branch 'main' into georgia-SEO-footer

d7561fd

Ensure works on mobile web and mobile

4c0f307

Update scrollViewContentContainerStyles

e180bb7

Fix not being able to click continue with keyboard open

785e369

Merge branch 'main' into georgia-SEO-footer

e0d06f8

Test using flex styling

01f02c8

Works on IOS, safari, android (-terms pushed up), not chrome

7c36de9

grgia mentioned this pull request Feb 24, 2023

Add SEO Footer to New Dot (All Platforms) #15022

Merged

64 tasks

Merge remote-tracking branch 'origin/main' into georgia-SEO-footer-flex

732182b

grgia added 3 commits February 25, 2023 00:44

Standardize keyboards. Allow drag to dismiss on mobile platforms, but…

6b5840a

… not web.

Code clean up

1b5c961

More clean up

23a2b83

tgolen requested changes Feb 25, 2023

View reviewed changes

grgia added 3 commits February 27, 2023 12:03

Move terms into login, clean up offline indicator

48eaa74

Add comments to explain code

4a06028

Remove unnecessary check

9f1c192

grgia closed this Feb 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

seo footer flex testing #15420

seo footer flex testing #15420

grgia commented Feb 23, 2023

grgia commented Feb 23, 2023

grgia commented Feb 23, 2023

grgia commented Feb 25, 2023

grgia commented Feb 25, 2023

tgolen Feb 25, 2023

tgolen Feb 25, 2023

tgolen Feb 25, 2023

tgolen Feb 25, 2023

grgia Feb 27, 2023

tgolen Feb 25, 2023

grgia commented Feb 27, 2023

grgia commented Feb 28, 2023

seo footer flex testing #15420

seo footer flex testing #15420

Conversation

grgia commented Feb 23, 2023

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

grgia commented Feb 23, 2023

grgia commented Feb 23, 2023

grgia commented Feb 25, 2023

grgia commented Feb 25, 2023

tgolen Feb 25, 2023

Choose a reason for hiding this comment

tgolen Feb 25, 2023

Choose a reason for hiding this comment

tgolen Feb 25, 2023

Choose a reason for hiding this comment

tgolen Feb 25, 2023

Choose a reason for hiding this comment

grgia Feb 27, 2023

Choose a reason for hiding this comment

tgolen Feb 25, 2023

Choose a reason for hiding this comment

grgia commented Feb 27, 2023

grgia commented Feb 28, 2023