Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.47k stars 2.82k forks source link

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

Closed kevinksullivan closed 4 weeks ago

kevinksullivan commented 1 month ago

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?

Screenshots/Videos

image

View all open jobs on GitHub

melvin-bot[bot] commented 1 month ago

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 commented 1 month ago

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 https://github.com/Expensify/App/blob/0a56250fc6da637737ceddec115ce5fbe233decb/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardPage.tsx#L46-L60 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 commented 1 month ago

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 https://github.com/Expensify/App/blob/488d5ff4739088e1c29bd31b6f9522dfd3b2d535/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardListPage.tsx#L108-L119

Optional: We can move headerButtons in WorkspaceExpensifyCardPage.tsx

What alternative solutions did you explore? (Optional)

github-actions[bot] commented 1 month ago

@No_action

VickyStash commented 1 month ago

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

allgandalf commented 1 month ago

Lets close this issue, PR was deployed