Add Download Banner to mWeb #25418

grgia · 2023-08-17T16:31:02Z

Requirements

Copied from E/E issue comment
Mockup:

User must dismiss modal to continue
Once dismissed, user will not see modal again
On clicking "Download", Opens App Store

Details

Adds a Download the App Banner for mWeb only. This is a part of the SaaStr project.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/304097

Tests

Verify that no errors appear in the JS console

NATIVE, WEB, DESKTOP

Verify Banner does NOT appear

mWEB

Verify Banner does appear
Click the close button, verify that Banner closes
mWEB IOS
Click the download button, verify that link opens Apple Store listing
mWEB Android
Click the download button, verify that link opens Google Play Store listing
Test Dismissing Banner
1. From Sign in page, dismiss banner. Sign in and verify that banner remains dismissed.
2. Sign out, verify that banner appears

mWEB Test Other Banners Were Not Affected by Styles

In Expensify.js modify this line:

                    {props.screenShareRequest ? (

to

                    {true ? (

Ensure modal looks like this:

In Expensify.js modify this line:

                    {props.updateAvailable ? <UpdateAppModal /> : null}

to

                    {true ? <UpdateAppModal /> : null}

Ensure modal looks like this:

Offline tests

QA Steps

Verify that no errors appear in the JS console
Verify that no errors appear in the JS console

NATIVE, WEB, DESKTOP

Verify Banner does NOT appear

mWEB

Verify Banner does appear
Click the close button, verify that Banner closes
mWEB IOS
Click the download button, verify that link opens Apple Store listing
mWEB Android
Click the download button, verify that link opens Google Play Store listing

Test Dismissing Banner

1. From Sign in page, dismiss banner. Sign in and verify that banner remains dismissed.
2. Sign out, verify that banner appears

PR Author Checklist

Screenshots/Videos

Web

Shouldnt Show

Mobile Web - Chrome

Mobile Web - Safari

Desktop

Shouldnt show

iOS

Shouldnt Show

Android

Shouldnt Show

melvin-bot · 2023-08-17T17:59:46Z

@jjcoffee Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

shawnborton · 2023-08-17T19:27:56Z

Looks good to me! Not sure if we need a period at the end of "Download the app." or not though?

And for the small text below the headline, I wonder if we should tighten up the lineHeight? Maybe try like 1.2 or something?

grgia · 2023-08-17T22:16:05Z

@jjcoffee fixed design comments, so this is ready for review!

jjcoffee · 2023-08-18T12:04:05Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible 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 checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (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 the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- 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
- 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 grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is 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 verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
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
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 code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
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 the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
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 reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

android-chrome-2023-08-23_18.05.01.mp4

Mobile Web - Safari

ios-safari-2023-08-23_17.37.03.mp4

Desktop

iOS

Android

dylanexpensify · 2023-08-18T14:18:42Z

@jjcoffee if you can prioritize this that would be amazing! High priority item for the team! 🙇‍♂️

jjcoffee · 2023-08-18T14:25:41Z

@dylanexpensify Sure, I'm on it! Should be done soon, just having some Android build problems but I think it's sorted now.

jjcoffee · 2023-08-18T14:37:04Z

@dylanexpensify I don't have access to the linked issue, are you able to verify for me that there was a copy review there? Just wondering about Download the App not strictly following the capitalisation guidelines.

grgia · 2023-08-18T14:47:20Z

Here's the mockup- good catch @jjcoffee! I'll change App to app

grgia · 2023-08-18T14:49:02Z

@jjcoffee pushed a fix

jjcoffee · 2023-08-18T15:14:13Z

@grgia Thanks for the quick fix! Can you confirm if there was a copy review on the original issue (I can't access it to check)?

grgia · 2023-08-18T15:25:07Z

@jjcoffee yep, marketing defined the copy!

jjcoffee · 2023-08-18T15:29:22Z

@grgia Is it expected behaviour that the banner will re-appear on reload? I'd imagine the intention would be to keep it closed for the duration of the session at least.

jjcoffee · 2023-08-18T15:35:53Z

@grgia Apart from the banner re-appering on reload, I've found a couple of issues whilst testing on Android Chrome.

If you dismiss the banner before signing in, it reappears after sign-in:

android-signing-in-issue.mp4

The banner blocks scrolling to the very bottom of the settings pane (you can't scroll to see "Sign out" in this case):

android-scroll-issue.mp4

It would be good to add a test to verify that the banner stays hidden once closed - on refresh (if we agree on the behaviour here), sign-in and on navigation once signed in.

grgia · 2023-08-23T15:18:26Z

@mountiny ready for review now

jjcoffee · 2023-08-23T15:27:32Z

@grgia Small one - there's a blue focus line around the screen whilst the banner is displayed (Android Chrome). Disappears after dismissing or tapping on the banner itself.

android-chrome-blue-focus-line.mp4

grgia · 2023-08-23T15:31:53Z

@jjcoffee That seems to be due to the tab selector and occurs for the other modals using this pattern

jjcoffee · 2023-08-23T15:45:44Z

@grgia I get this when I try and open the download link on Safari iOS. The link opens fine if I copy it from source into Chrome, so might just be an issue with the emulator.

grgia · 2023-08-23T15:52:08Z

@jjcoffee yeah the emulators don't have the app store downloaded, but the adHoc web test passes on my physical device!

jjcoffee

Looks good and tests well!

melvin-bot · 2023-08-23T16:12:31Z

@dangrous Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot · 2023-08-23T16:12:35Z

🎯 @jjcoffee, thanks for reviewing and testing this PR! 🎉

An E/App issue has been created to issue payment here: #25783.

melvin-bot · 2023-08-23T16:50:04Z

@deetergp Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

mountiny · 2023-08-23T17:40:21Z

looks good to me

shawnborton · 2023-08-23T17:57:01Z

Looks good to me too!

mountiny

@grgia Great job on this one and @jjcoffee for reviewing and testing 🙇

Please dont forget to make an issue to track the follow ups

mountiny · 2023-08-23T18:22:46Z

src/components/ConfirmContent.js

@@ -111,7 +117,27 @@ function ConfirmContent(props) {
                {_.isString(props.prompt) ? <Text style={[...props.promptStyles, isCentered ? styles.textAlignCenter : {}]}>{props.prompt}</Text> : props.prompt}
            </View>

-            {isCentered ? (
+            {props.shouldStackButtons ? (


Could we make an issue to track this change otherwise we never gonna update it 😂

mountiny · 2023-08-23T18:23:17Z

src/components/DownloadAppModal.js

+import ONYXKEYS from '../ONYXKEYS';
+import styles from '../styles/styles';
+import CONST from '../CONST';
+import AppIcon from '../../assets/images/expensify-app-icon.svg';


Could we also make an issue for this? @grgia

mountiny · 2023-08-23T18:23:49Z

src/languages/en.js

@@ -246,6 +246,11 @@ export default {
        newFaceEnterMagicCode: ({login}) => `It's always great to see a new face around here! Please enter the magic code sent to ${login}. It should arrive within a minute or two.`,
        welcomeEnterMagicCode: ({login}) => `Please enter the magic code sent to ${login}. It should arrive within a minute or two.`,
    },
+    DownloadAppModal: {


I dont think other keys are capitalized

Suggested change

DownloadAppModal: {

downloadAppModal: {

OSBotify · 2023-08-23T18:29:06Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2023-08-24T03:35:10Z

🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.57-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

grgia · 2023-08-24T09:39:16Z

I created a issue to handle these follow up changes on a lower-priority basis #25820

OSBotify · 2023-08-28T18:51:50Z

🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.58-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-08-29T00:06:51Z

🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.57-6 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-08-30T01:20:53Z

🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.58-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

Add Mobile Banner Component, Styled (not functional)

d20ba5e

grgia self-assigned this Aug 17, 2023

grgia added 4 commits August 17, 2023 12:11

Clean up and add functionality

ecd7d39

Move out of folder

0c24cfa

fix asset path

aefdaeb

Add copy to en/es

8d14fa3

grgia changed the title ~~Add Mobile Banner Component, Styled (not functional)~~ Add Download Banner to mWeb Aug 17, 2023

grgia added 2 commits August 17, 2023 12:45

modify spanish translation from slack feedback

fde38d5

Fix lint

e61e3f0

grgia marked this pull request as ready for review August 17, 2023 17:59

grgia requested a review from a team as a code owner August 17, 2023 17:59

melvin-bot bot requested review from jjcoffee and removed request for a team August 17, 2023 17:59

grgia requested a review from shawnborton August 17, 2023 18:03

grgia added 2 commits August 17, 2023 17:14

Design comments- remove period, fix line height in second line

81f1536

Merge branch 'main' into georgia-DownloadAppBanner

b19d81d

Fix Capitalization of 'Download the App' to 'Download the app'

b415c22

jjcoffee approved these changes Aug 23, 2023

View reviewed changes

melvin-bot bot requested a review from dangrous August 23, 2023 16:12

melvin-bot bot mentioned this pull request Aug 23, 2023

Payment issue for E/App PR#25418 #25783

Closed

grgia requested a review from jjcoffee August 23, 2023 16:21

jjcoffee approved these changes Aug 23, 2023

View reviewed changes

melvin-bot bot requested a review from deetergp August 23, 2023 16:50

shawnborton approved these changes Aug 23, 2023

View reviewed changes

mountiny approved these changes Aug 23, 2023

View reviewed changes

mountiny merged commit 76d72e4 into main Aug 23, 2023
15 of 18 checks passed

mountiny deleted the georgia-DownloadAppBanner branch August 23, 2023 18:24

OSBotify mentioned this pull request Aug 24, 2023

Deploy Checklist: New Expensify 2023-08-24 #25813

Closed

73 tasks

grgia mentioned this pull request Aug 24, 2023

Clean up for mWeb Download App Banner #25820

Closed

OSBotify mentioned this pull request Aug 28, 2023

Deploy Checklist: New Expensify 2023-08-28 #26113

Closed

grgia mentioned this pull request Aug 29, 2023

Don't show the Download App Banner until Past Sign in #26182

Merged

73 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Download Banner to mWeb #25418

Add Download Banner to mWeb #25418

grgia commented Aug 17, 2023 •

edited

Loading

melvin-bot bot commented Aug 17, 2023

shawnborton commented Aug 17, 2023

grgia commented Aug 17, 2023

jjcoffee commented Aug 18, 2023 •

edited

Loading

dylanexpensify commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

grgia commented Aug 18, 2023

grgia commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

grgia commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

jjcoffee commented Aug 18, 2023 •

edited

Loading

grgia commented Aug 23, 2023

jjcoffee commented Aug 23, 2023 •

edited

Loading

grgia commented Aug 23, 2023

jjcoffee commented Aug 23, 2023

grgia commented Aug 23, 2023

jjcoffee left a comment

melvin-bot bot commented Aug 23, 2023

melvin-bot bot commented Aug 23, 2023

melvin-bot bot commented Aug 23, 2023

mountiny commented Aug 23, 2023

shawnborton commented Aug 23, 2023

mountiny left a comment

mountiny Aug 23, 2023

mountiny Aug 23, 2023

mountiny Aug 23, 2023

OSBotify commented Aug 23, 2023

OSBotify commented Aug 24, 2023

grgia commented Aug 24, 2023

OSBotify commented Aug 28, 2023

OSBotify commented Aug 29, 2023

OSBotify commented Aug 30, 2023

Add Download Banner to mWeb #25418

Add Download Banner to mWeb #25418

Conversation

grgia commented Aug 17, 2023 • edited Loading

Requirements

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Aug 17, 2023

shawnborton commented Aug 17, 2023

grgia commented Aug 17, 2023

jjcoffee commented Aug 18, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

dylanexpensify commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

grgia commented Aug 18, 2023

grgia commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

grgia commented Aug 18, 2023

jjcoffee commented Aug 18, 2023

jjcoffee commented Aug 18, 2023 • edited Loading

grgia commented Aug 23, 2023

jjcoffee commented Aug 23, 2023 • edited Loading

grgia commented Aug 23, 2023

jjcoffee commented Aug 23, 2023

grgia commented Aug 23, 2023

jjcoffee left a comment

Choose a reason for hiding this comment

melvin-bot bot commented Aug 23, 2023

melvin-bot bot commented Aug 23, 2023

melvin-bot bot commented Aug 23, 2023

mountiny commented Aug 23, 2023

shawnborton commented Aug 23, 2023

mountiny left a comment

Choose a reason for hiding this comment

mountiny Aug 23, 2023

Choose a reason for hiding this comment

mountiny Aug 23, 2023

Choose a reason for hiding this comment

mountiny Aug 23, 2023

Choose a reason for hiding this comment

OSBotify commented Aug 23, 2023

OSBotify commented Aug 24, 2023

grgia commented Aug 24, 2023

OSBotify commented Aug 28, 2023

OSBotify commented Aug 29, 2023

OSBotify commented Aug 30, 2023

grgia commented Aug 17, 2023 •

edited

Loading

jjcoffee commented Aug 18, 2023 •

edited

Loading

jjcoffee commented Aug 18, 2023 •

edited

Loading

jjcoffee commented Aug 23, 2023 •

edited

Loading