[$250] Update the workspace connected bank account page for clarity and consistency

[anmurali]

Users are confused when they go to add a bank account to their workspace and cannot select their linked personal bank accounts. So let's add some explainer text that educates them on the need for a Business Bank Account. While we're at it, let's also update the page to our newest pattern for consistency.

Today

Update to

Final copy for the note:

Note: Personal bank accounts can't be used for payments on workspaces.

cc @shawnborton @maddylewis @jamesdeanexpensify

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a8923a1af5c75af2
• Upwork Job ID: 1824502151817768637
• Last Price Increase: 2024-08-23
• Automatic offers:
• BhuvaneshPatil | Contributor | 103689249

Issue Owner

Current Issue Owner: @Ollyws

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a8923a1af5c75af2

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws (External)

[cretadn22]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update the workspace connected bank account page for clarity and consistency (New design)

What is the root cause of that problem?

New design

What changes do you think we should make in order to solve the problem?

We need to update this Section Component

App/src/pages/ReimbursementAccount/BankAccountStep.tsx

Line 128 in 279f561

<Section

to

<Section
       title={translate('workspace.bankAccount.streamlinePayments')}
       subtitle={translate('bankAccount.toGetStarted')}
       subtitleMuted
       illustration={LottieAnimations.Safe}.  // Need to use the new design 
       titleStyles={styles.titleStyle}  // Need to define
       childrenStyles={styles.childrenStyles}  // Need to define
>

Also need to add new note (detai in the PR)

What alternative solutions did you explore? (Optional)

[Nodebrute]

Edited by proposal-police: This proposal was edited at 2024-08-17T14:15:00Z.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add a note

What is the root cause of that problem?

Feature request

What changes do you think we should make in order to solve the problem?

We can add following styles here. We should conditionally render these styles and display this note only when the user has added a personal bank account. We can use this key to check if plaidAccountID is stored in Onyx.

 key: ONYXKEYS.PERSONAL_BANK_ACCOUNT

     <View style={[styles.flexRow, styles.mb4, styles.alignItemsCenter, styles.pb1, styles.pt1]}>
                        <Icon
                            src={Expensicons.Lightbulb}
                            fill={theme.icon}
                            additionalStyles={styles.mr2}
                            small
                        />
                        <Text
                            style={[styles.textLabelSupportingNormal]}
                            suppressHighlighting
                        >
                            {translate('note')} // we can add note here
                        </Text>
                    </View>

We should also add the note in en.ts and es.ts file

Further styles can be modified according to the design in the PR.

We can update this section here. Additionally, I think we should add the illustrationBackgroundColor to align with the design.

 <Section
                        illustration={LottieAnimations.FastMoney} //Animation of our choice
                        title={translate('workspace.bankAccount.streamlinePayments')}
                        subtitle={translate('bankAccount.toGetStarted')}
                        subtitleMuted
                    >

Note

As noted by a fellow contributor regarding the use of isCentralPane, I believe we should avoid using it. This prop is specifically for when the section is located in the central pane of the layout.

The main issue here is that we’re only checking isCentralPane, but we should also be checking for illustration. We need to update the check to (isCentralPane || !!illustration).

App/src/components/Section/index.tsx

Line 173 in 7615898

<View style={[styles.w100, isCentralPane && (shouldUseNarrowLayout ? styles.p5 : contentPaddingOnLargeScreens ?? styles.p8)]}>

Additionally, I noticed that the design uses the same styles for 'Connect Manually' and 'Connect with Plaid'. Therefore, we should also update the styles for 'Connect with Plaid' to match.

All the font sizes and other styles can be further adjusted in the pull request.

What alternative solutions did you explore? (Optional)

[BhuvaneshPatil]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update the workspace connected bank account page for clarity and consistency [Design Update]

What is the root cause of that problem?

New Feature

What changes do you think we should make in order to solve the problem?

1. We should use Section component's prop to apply new design.
   

   App/src/pages/ReimbursementAccount/BankAccountStep.tsx

   Lines 128 to 131 in c7d4c5f

   	<Section
   	icon={Illustrations.MoneyWings}
   	title={translate('workspace.bankAccount.streamlinePayments')}
   	>

<Section
  title={translate("workspace.bankAccount.streamlinePayments")}
  illustration={LottieAnimations.Coin}
  isCentralPane
  subtitle={translate("bankAccount.toGetStarted")}
  subtitleMuted
></Section>;

2. By passing illustration instead of icon, we can achieve given design. With this prop, we need to add extra prop isCentralPane (which is missing in both proposals), and if it's passed the content doesn't have padding. With these props we need to pass subtitle prop as well, with subtitleMuted.
3. For showing new content (one with Bulb icon in design), we will need to add following view - Inside
   component (styles can be modified according to design)

<View
  style={[
    styles.flexRow,
    styles.mv3,
    styles.alignItemsStart,
    styles.gap2,
    styles.mln1,
  ]}
>
  <Icon
    src={Expensicons.Lightbulb}
    height={variables.iconSizeLarge}
    width={variables.iconSizeLarge}
    fill={theme.icon}
    additionalStyles={styles.p1}
  />
  <Text style={[styles.textLabelSupporting, styles.textAlignLeft]}>
    Lorem ipsum dolor sit amet consectetur adipisicing elit. Eligendi fugiat
    facilis nisi alias
  </Text>
</View>;

Result -

Without isCentralPane

What alternative solutions did you explore? (Optional)

[shawnborton]

Just adding a note that we do have an existing lottie animation to use in the top green area of the card, so it will match other places in our app (like in Settings) where the header area is animated.

[Ollyws]

Looking at the timestamps of the proposal edits it seems that @BhuvaneshPatil had the first fully fleshed-out proposal so lets go with them.
🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Nodebrute]

@Ollyws When @BhuvaneshPatil proposed the solution, the only difference between his proposal and mine was the usage of isCentralPane, which was incorrect in his proposal. The main difference between our proposals was indeed the usage of isCentralPane. I later pointed out the correct root cause analysis (RCA). Could you please review the proposal again?

[Nodebrute]

Hey @Ollyws, I wanted to clarify that isCentralPane isn't applicable here, as it is specifically meant for central panel layouts. I was the first to propose the note code and subsequently suggested the correct approach for adding animation.

[BhuvaneshPatil]

@Nodebrute By looking at timestamps, when I posted my proposal, your proposal didn't mention about updating /adding illustration prop in. <Section/> component

[Nodebrute]

@BhuvaneshPatil Hey, I missed that we were also supposed to add the illustration. I've since updated my code to include the illustrations, but my approach is quite different from yours. Just like in your proposal, it looks like you missed mentioning the 'Connect Online with Plaid' button

[Nodebrute]

Hey @Ollyws, here's a timeline of all the proposals:

• @cretadn22 was the first to mention the section code.
• I then introduced the note code.
• @BhuvaneshPatil's proposal addressed both the section and note codes and pointed out that the isCentralPane was missing from the previous proposals.
• I subsequently updated my proposal by adding a note explaining why we should avoid using isCentralPane and correct section code. I also added the change for the 'Connect Online with Plaid' button.

[Nodebrute]

Hi @Ollyws, just a gentle reminder to let me know your thoughts on this when you have a moment. Thanks!

[tgolen]

Hey, I'm just seeing this issue, but whoever works on this, can you please only make the new "Note" visible when the user has a personal bank account connected? I don't think the note needs to be visible if they haven't ever added a personal bank account and it could be just as confusing.

[Nodebrute]

Proposal updated with the new requested changes.

[melvin-bot]

@francoisl, @Ollyws Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[francoisl]

Hi @Ollyws, can confirm the chosen proposal is still valid please, especially given the last request to only make the note visible after an account is connected, and also the discussion about using isCentralPane above in this comment?

[Ollyws]

I still stand by my initial selection of @BhuvaneshPatil

It was the first and only proposal that suggested adding all the required features including the illustration prop.
Although it was the second proposal to outline usage of the Section component, the first proposal to mention it would have caused a regression due to lack of the isCentralPane prop.

@Nodebrute then built upon @BhuvaneshPatil’s proposal suggesting we further modify what is essentially the functionality of the isCentralPane prop to check for the illustration, while it may be a nice addition it does not constitute a new, unique proposal in itself.

As for this note, it was added after I had selected the proposal so it can be considered a note addressed to the selected contributor for consideration in their PR.

[melvin-bot]

📣 @BhuvaneshPatil 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[trjExpensify]

Adding the Bug label to get a BZ assigned. Also, @BhuvaneshPatil can you provide an ETA for the PR? Thanks!

[melvin-bot]

@francoisl, @Ollyws, @BhuvaneshPatil Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@francoisl @Ollyws @BhuvaneshPatil @kadiealexander this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[BhuvaneshPatil]

@trjExpensify I am facing trouble testing on IOS device. I will raise PR as soon as it works

[trjExpensify]

Have you taken your questions to #expensify-open-source to get some help on what's blocking you from testing on iOS?

[maddylewis]

@BhuvaneshPatil - bump on Tom's question when you have a sec! #47578 (comment)

[BhuvaneshPatil]

PR is in review

[BhuvaneshPatil]

@anmurali

While working on PR I found out that this page is also using old design shall we cover this page as well.

I would love to work on that as well. Let me know what you think.

[shawnborton]

I think following up to fix that particular spot makes sense for consistency's sake.

I also think something we missed when implementing this is that we should use our standard edge-to-edge push rows here. Right now we use the old "rounded pill" option row style which I think we should kill:

CleanShot.2024-09-13.at.07.25.38.mp4

The correct style looks like this:

CleanShot.2024-09-13.at.07.26.46.mp4

[trjExpensify]

@BhuvaneshPatil can you work on a PR to address Shawn's feedback please, and then tag Design for review? Thanks!

[BhuvaneshPatil]

Sure. I will raise the PR.

@trjExpensify what do you think about this comment

[BhuvaneshPatil]

raised the PR, uploaded video for macos web. will update videos on other platforms next morning

[trjExpensify]

I agree with Shawn:

I think following up to fix that particular spot makes sense for consistency's sake.

[BhuvaneshPatil]

@trjExpensify @shawnborton PR is now merged 🎊

[trjExpensify]

Great!