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

[Workspace Feeds] Double header on the Expensify Card #48944

Closed
1 of 6 tasks
kevinksullivan opened this issue Sep 10, 2024 · 6 comments
Closed
1 of 6 tasks

[Workspace Feeds] Double header on the Expensify Card #48944

kevinksullivan opened this issue Sep 10, 2024 · 6 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Reviewing Has a PR in review

Comments

@kevinksullivan
Copy link
Contributor

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number:
Reproducible in staging?:
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

Action Performed:

  1. Add account to workspaceFeeds beta
  2. Sign in with test account
  3. Select Manage my team’s expenses
  4. Enter any business name / personal name
  5. Navigate to the workspace editor of the newly created workspace
  6. Navigate to More features tab
  7. Toggle on the feature
  8. Navigate to the Expensify Card page
  9. Select Issue new card, give the user a $2 limit
  10. Follow the steps for an OPEN bank account (option 3) of this SO
  11. Finish the setup process
  12. Issue a card
  13. Navigate back to the Expensify Card page

Expected Result:

Only see one Expensify Card header.

Actual Result:

Two Expensify Card headers are shown.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

image

View all open jobs on GitHub

@kevinksullivan kevinksullivan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 10, 2024
Copy link

melvin-bot bot commented Sep 10, 2024

Triggered auto assignment to @dylanexpensify (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.

@abzokhattab
Copy link
Contributor

abzokhattab commented Sep 10, 2024

Proposal

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

Double header on the Expensify Card

What is the root cause of that problem?

A regression from this PR where here

<WorkspacePageWithSections
shouldUseScrollView
icon={Illustrations.HandCard}
headerText={translate('workspace.common.expensifyCard')}
route={route}
guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_EXPENSIFY_CARD}
shouldShowOfflineIndicatorInWideScreen
isLoading={isLoading}
>
{!!paymentBankAccountID && !isLoading && (
<WorkspaceExpensifyCardListPage
cardsList={cardsList}
route={route}
/>
)}
both the WorkspacePageWithSectionsand the WorkspaceExpensifyCardListPage have the header component

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

we should move the WorkspaceExpensifyCardListPage outside the WorkspacePageWithSections

            {!!paymentBankAccountID && !isLoading && (
                <WorkspaceExpensifyCardListPage
                    cardsList={cardsList}
                    route={route}
                />
            )}

            {!paymentBankAccountID && !isLoading && (
                <WorkspacePageWithSections
                    shouldUseScrollView
                    icon={Illustrations.HandCard}
                    headerText={translate('workspace.common.expensifyCard')}
                    route={route}
                    guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_EXPENSIFY_CARD}
                    shouldShowOfflineIndicatorInWideScreen
                    isLoading={isLoading}
                >
                    <WorkspaceExpensifyCardPageEmptyState route={route} />
                </WorkspacePageWithSections>
            )}

POC

image

@Nodebrute
Copy link
Contributor

Nodebrute commented Sep 10, 2024

Proposal

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

Double header on the Expensify Card

What is the root cause of that problem?

We are adding headerContent twice once here in WorkspaceExpensifyCardPage.tsx and then again in WorkspaceExpensifyCardListPage.tsx

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

We can remove HeaderWithBackButton and ScreenWrapper from here

<ScreenWrapper
shouldEnablePickerAvoiding={false}
shouldShowOfflineIndicatorInWideScreen
shouldEnableMaxHeight
testID={WorkspaceExpensifyCardListPage.displayName}
>
<HeaderWithBackButton
icon={Illustrations.HandCard}
title={translate('workspace.common.expensifyCard')}
shouldShowBackButton={shouldUseNarrowLayout}
onBackButtonPress={() => Navigation.goBack()}
>

Optional: We can move headerButtons in WorkspaceExpensifyCardPage.tsx

What alternative solutions did you explore? (Optional)

Copy link
Contributor

@No_action

@VickyStash
Copy link
Contributor

Hey, I'm Viktoryia from Callstack, I've been working on this page, so I can take a look.

@allgandalf
Copy link
Contributor

Lets close this issue, PR was deployed

@allgandalf allgandalf removed their assignment Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

8 participants