Skip to content

Corrected floating button in VBA flow. Now button shows at bottom of each step. - #5208

Merged
roryabraham merged 3 commits into
Expensify:mainfrom
PrashantMangukiya:prashant-5183
Sep 15, 2021
Merged

roryabraham merged 3 commits into
Expensify:mainfrom
PrashantMangukiya:prashant-5183

Conversation

@PrashantMangukiya

@PrashantMangukiya PrashantMangukiya commented Sep 10, 2021 •

Copy link
Copy Markdown
Contributor

@roryabraham pull request ready for review, if anything missing then kindly inform me, I will update it.

Details

Corrected floating button in VBA flow, so now button shows at bottom of each page content end. So for long form user can focus on fill up information and see button when reach at the bottom. Only Ui related changes done without affecting logical part. i.e. moved floating button to bottom of each page within a VBA flow.

Fixed Issues

$ #5183

Tests

  1. Create Workspace
  2. Select Get Started
  3. Choose Connect Manually option to add bank account via routing/account number, continue
  4. See the Save & Continue option regardless of scroll position on the Company Information step
    To check ui for individual page within flow, we can open related route from localhost.

QA Steps

Each page within VBA flow have floating button moved to end of form/content so each page look consistent. At present some issue to fillup the form and test vba steps, so each page browsed from localhost via route and checked that button correctly at bottom of page content.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Web1

Web2A

Web2B

Web3

Web4

Mobile Web

MobileWeb1



MobileWeb2A




MobileWeb2B




MobileWeb4

Desktop

Desktop1

Desktop2A

Desktop2B

Desktop3

Desktop4

iOS

iOS1

iOS2A

iOS2B

iOS3

iOS4

Android

Android1

Android2A

Android2B

Android3

Android4

@PrashantMangukiya
PrashantMangukiya requested a review from a team as a code owner September 10, 2021 21:20
@MelvinBot
MelvinBot requested review from roryabraham and removed request for a team September 10, 2021 21:20
@roryabraham

Copy link
Copy Markdown
Contributor

@Expensify/design Should the button be on the bottom of the screen, even if you have to scroll down to the bottom to see it?

Also @PrashantMangukiya I don't think I'll be able to review this until Sunday or Monday.

@michelle-thompson

Copy link
Copy Markdown
Contributor

Yes, the button should be bottom-docked

@shawnborton

Copy link
Copy Markdown
Contributor

I agree with Michelle. We can probably accomplish this with some flex grow magic is my guess.

@roryabraham

Copy link
Copy Markdown
Contributor

Thanks @michelle-thompson and @shawnborton, that was my thought too!

@PrashantMangukiya Let us know if you have any questions.

@marcaaron

Copy link
Copy Markdown
Contributor

Just to confirm we are asking that if the entire form only takes up the partial height of the screen push the button to the bottom? Otherwise if the form takes up more than the full height of the screen then the button should already be at the bottom when the form is scrolled?

@shawnborton

Copy link
Copy Markdown
Contributor

Yup, that sounds right @marcaaron

isDisabled={!this.canSubmitManually()}
onPress={this.addManualAccount}
/>
</View>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know there's not a lot going on in this step - but I think this should be a ScrollView?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replaced View with ScrollView as per request, also adjusted style accordingly.

@PrashantMangukiya

Copy link
Copy Markdown
Contributor Author

Hi @marcaaron I pushed new commit that includes changes you have suggested. Thank you for your time.

@roryabraham

Copy link
Copy Markdown
Contributor

@PrashantMangukiya It seems like you haven't yet made this change:

if the entire form only takes up the partial height of the screen, push the button to the bottom. Otherwise, if the form takes up more than the full height of the screen, then the button should already be at the bottom when the form is scrolled.

Please update the PR screenshots and let us know when that change has been made.

@PrashantMangukiya

Copy link
Copy Markdown
Contributor Author

@roryabraham, Sorry for confusion to me, I though we are discussing and not finalised yet, so I did not implemented. I am making changes accordingly and submitting new commit and update pr screenshot soon. Thanks.

@marcaaron

Copy link
Copy Markdown
Contributor

@PrashantMangukiya I think this solution might work:

  • Make the ScrollView have styles like this:
            <ScrollView
                style={[styles.w100, styles.flex1]}
                contentContainerStyle={styles.flexGrow1}
            >
  • The button can then be wrapped in a View with a style of flex1.

FWIW I solved it this way in another PR here that we will merge after this one (but haven't yet tested the solution on native platforms)

…hows at bottom of content

If the entire form only takes up the partial height of the screen, push the button to the bottom. Otherwise, if the form takes up more than the full height of the screen, then the button should already be at the bottom when the form is scrolled.
@PrashantMangukiya

Copy link
Copy Markdown
Contributor Author

@roryabraham Changes made as requested i.e. New commit pushed, and pr screenshot also updated. @marcaaron thank you for suggestion. You can proceed for pr review.

@marcaaron marcaaron left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please test on Android & iOS and provide screenshots. I don't see any reason to mark these as N/A

@PrashantMangukiya

Copy link
Copy Markdown
Contributor Author

@marcaaron as mentioned in the #5183 (comment) vba flow cant run via form input at present and it has some open issue etc. So not able to fill up form and run complete flow in iOS and android, so screenshot not produced. So due to that limitation we used direct link as shown below to test updated ui at locally. Because in this issue only button related ui changes was requested.

@marcaaron

Copy link
Copy Markdown
Contributor

Try using deep links?

@marcaaron

Copy link
Copy Markdown
Contributor

I think you can just send yourself a message like this then click on it and it should open

2021-09-14_12-24-45

@PrashantMangukiya

Copy link
Copy Markdown
Contributor Author

@marcaaron , thanks for suggesting deep link, let me try that way to create screenshot for iOS and Android if I can do. Thanks.

@PrashantMangukiya

Copy link
Copy Markdown
Contributor Author

@marcaaron, Attached is the screen shot for iOS, Android and Desktop. Thank you again for deep link suggestion.

@marcaaron marcaaron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This LGTM. Normally I might ask to see if we can reduce some duplication around the styles in the ScrollView, but I'm already doing this work in another issue so I think what we have here is 👌 for now.

@roryabraham roryabraham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@roryabraham
roryabraham merged commit 4e5d435 into Expensify:main Sep 15, 2021
@OSBotify

Copy link
Copy Markdown
Contributor

✋ 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 OSBotify mentioned this pull request Sep 15, 2021
27 tasks done
@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.0.98-2 🚀

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

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @Jag96 in version: 1.0.99-4 🚀

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

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.

6 participants