[$250] Billable-Billable and "members must categorize all expense" on toggle off text color behavior

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: V9. 0.70-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Issue reported by: Applause Internal Team

Action Performed:

1. Launch app
2. Open a workspace chat with billable option enabled
3. Try to create a expense
4. Note in confirmation page, all option displayed in different color and billable option in different color
5. Toggle off billable option
6. Note now it matches with other option colors
7. Navigate to workspace settings - category- setti gs
8. Note the "members must categorize all expense" And below category heading text color is different
9. Toggle on/off "members must categorize all expense"
10. Note text color doesn't changes

Expected Result:

Billable and "members must categorize all expense" on toggle off text color behavior must be consistent.

Actual Result:

Billable and "members must categorize all expense" on toggle off text color behavior is inconsistent.

Workaround:

Unknown

Platforms:

• [x] Android: Standalone
• [x] Android: HybridApp
• [x] Android: mWeb Chrome
• [ ] iOS: Standalone
• [ ] iOS: HybridApp
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/54d5d2f2-5241-45f9-8eb2-d26456f9484f

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021865173764798215374
• Upwork Job ID: 1865173764798215374
• Last Price Increase: 2024-12-06

Issue Owner

Current Issue Owner: @ikevin127

[melvin-bot[bot]]

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

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-12-03 14:06:05 UTC.

Proposal

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

Billable-Billable and "members must categorize all expense" on toggle off text color behavior

What is the root cause of that problem?

We are only applying the textSupporting color style when the iouBillable is false https://github.com/Expensify/App/blob/64829131fe962681626b4b016140521d05f7fe52/src/components/MoneyRequestConfirmationListFooter.tsx#L532

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

According to the comment here we should not fade out on toggle off so we can remove textSupporting styles here https://github.com/Expensify/App/blob/64829131fe962681626b4b016140521d05f7fe52/src/components/MoneyRequestConfirmationListFooter.tsx#L532 and https://github.com/Expensify/App/blob/48b80f65e80d304965cbeb3573d7d6678fb9bd3a/src/components/ReportActionItem/MoneyRequestView.tsx#L733 and to make the whole page scrollable as pointed out in the comment we can include both the ToggleSettingOptionRow and selection list here into a scroll view like here

                <ScrollView contentContainerStyle={[styles.flexGrow1]}>

we can also take ToggleSettingOptionRow into listHeaderContent of the selection list but I don't think it is appropriate as the requireCategory toggle setting is a separate setting from the selection list settings.

or We should always apply the text supporting style unconditionally

                        titleStyle={{color: theme.textSupporting}}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

If we want we can also change title style here to change color based on the stte (apply text supporting when policy?.requiresCategory is false ) https://github.com/Expensify/App/blob/64829131fe962681626b4b016140521d05f7fe52/src/pages/workspace/categories/WorkspaceCategoriesSettingsPage.tsx#L103 but I don't think that is preferable

What alternative solutions did you explore? (Optional)

[saifelance]

Proposal

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

When the "Billable" toggle is turned off, the text color changes to match other options. However, when toggling the "members must categorize all expenses" setting, the text color doesn't change consistently.

What is the root cause of that problem?

• Consistent State Handling: Ensure that the toggle states for both "Billable" and "members must categorize all expenses"

• Styles Consistency:

• For the ToggleSettingOptionRow used in both "Billable" and "members must categorize all expenses," ensure that a consistent style is applied based on whether the toggle is on or off.

• Use the same conditional logic for text color change as applied in the "Billable" toggle for the "members must categorize all expenses" toggle.

The existing logic applies a style change when iouIsBillable is false. You can apply similar logic to the "members must categorize all expenses" toggle.

src/components/MoneyRequestConfirmationListFooter.tsx

const membersCategorizeStyle = {
    color: isMembersCategorizeActive ? theme.textPrimary : theme.textSupporting,
};

// Apply this style in the "members must categorize all expenses" section.
<MenuItemWithTopDescription
    key={translate('common.membersMustCategorize')}
    title={translate('common.membersMustCategorize')}
    description={translate('common.membersMustCategorizeDescription')}
    style={[styles.moneyRequestMenuItem, membersCategorizeStyle]} // Apply the consistent style
/>

When the "Billable" toggle is switched off, you already apply titleStyle={!iouIsBillable && {color: theme.textSupporting}}. Ensure that the style for "members must categorize all expenses" is handled similarly in the style prop based on the toggle state

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-12-09 04:20:04 UTC.

Proposal

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

Billable-Billable and "members must categorize all expense" on toggle off text color behavior

What is the root cause of that problem?

• theme.textSupporting color is applied conditionally to the text. https://github.com/Expensify/App/blob/0c13d2aa3e187f7a76d20e51b3b3edb4440e6e3a/src/components/ReportActionItem/MoneyRequestView.tsx#L733 https://github.com/Expensify/App/blob/01d5233e80a4d4dbe87e6b1e85fdea5dc3c10136/src/components/MoneyRequestConfirmationListFooter.tsx#L532

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

• Since billable is a description, we should maintain consistency with the other field's description colors in the MoneyRequestView and MoneyRequestConfirmationListFooter components. To achieve this, we can remove the condition from both places and always apply the theme.textSupporting color.
• For making the whole view scrollable, we need to use listHeaderContent instead of headerContent and update the headerContent to include also ToggleSettingOptionRow & divider line. https://github.com/Expensify/App/blob/7925c9b555af60156c62947ae63b517d6c31587f/src/pages/workspace/categories/WorkspaceCategoriesSettingsPage.tsx#L102-L124

Updated code

        <AccessOrNotFoundWrapper
            policyID={policyID}
            accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
            featureName={CONST.POLICY.MORE_FEATURES.ARE_CATEGORIES_ENABLED}
        >
            <ScreenWrapper
                includeSafeAreaPaddingBottom={false}
                style={[styles.defaultModalContainer]}
                testID={WorkspaceCategoriesSettingsPage.displayName}
            >
                <HeaderWithBackButton
                    title={translate('common.settings')}
                    onBackButtonPress={() => Navigation.goBack(isQuickSettingsFlow ? ROUTES.SETTINGS_CATEGORIES_ROOT.getRoute(policyID, backTo) : undefined)}
                />

                <View style={[styles.containerWithSpaceBetween]}>
                    {!!currentPolicy && (sections.at(0)?.data?.length ?? 0) > 0 && (
                        <SelectionList
                            listHeaderContent={
                                <View>
                                    <ToggleSettingOptionRow
                                        title={translate('workspace.categories.requiresCategory')}
                                        subtitle={toggleSubtitle}
                                        switchAccessibilityLabel={translate('workspace.categories.requiresCategory')}
                                        isActive={policy?.requiresCategory ?? false}
                                        onToggle={updateWorkspaceRequiresCategory}
                                        pendingAction={policy?.pendingFields?.requiresCategory}
                                        disabled={isToggleDisabled}
                                        wrapperStyle={[styles.pv2, styles.mh5]}
                                        errors={policy?.errorFields?.requiresCategory ?? undefined}
                                        onCloseError={() => Policy.clearPolicyErrorField(policy?.id ?? '-1', 'requiresCategory')}
                                        shouldPlaceSubtitleBelowSwitch
                                    />
                                    <View style={[styles.sectionDividerLine]} />
                                    <View style={[styles.mh5, styles.mt2, styles.mb1]}>
                                        <Text style={[styles.headerText]}>{translate('workspace.categories.defaultSpendCategories')}</Text>
                                        <Text style={[styles.mt1, styles.lh20]}>{translate('workspace.categories.spendCategoriesDescription')}</Text>
                                    </View>
                                </View>
                            }
                            sections={sections}
                            ListItem={SpendCategorySelectorListItem}
                            onSelectRow={(item) => {
                                if (!item.groupID || !item.categoryID) {
                                    return;
                                }
                                setIsSelectorModalVisible(true);
                                setCategoryID(item.categoryID);
                                setGroupID(item.groupID);
                            }}
                        />
                    )}
                    {!!categoryID && !!groupID && (
                        <CategorySelectorModal
                            policyID={policyID}
                            isVisible={isSelectorModalVisible}
                            currentCategory={categoryID}
                            onClose={() => setIsSelectorModalVisible(false)}
                            onCategorySelected={setNewCategory}
                            label={groupID[0].toUpperCase() + groupID.slice(1)}
                        />
                    )}
                </View>
            </ScreenWrapper>
        </AccessOrNotFoundWrapper>

- We should also ensure that on other similar pages we are using `listHeaderContent` instead of `headerContent`.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?


What alternative solutions did you explore? (Optional)

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ikevin127]

From my understanding of the issue here is that while the switch inputs are toggled OFF the Billable label should be the same as the Members must categorize all expenses label (meaning black), since toggled ON are the same currently.

Billable and "members must categorize all expense" on toggle off text color behavior must be consistent.

Since the Expected result is a bit vague, I'd want to ask @Expensify/design's take on what consistent should mean in this issues case for both Billable and Members must categorize all expenses before making a decision on the proposals:

Billable OFF	Members OFF

Billable ON	Members OFF

Billable w/ Context	Members w/ Context

The reason why I want design's take on this is because if we look at the w/ Context images we can notice that in both cases all other input labels are grey with their values being black (if existent), but in the Billable case the switch toggle label is grey when OFF and black when ON, while in the Members case the switch toggle label is always black even though in both cases all other input labels are grey with their values being black (if existent).

[dubielzyk-expensify]

From my understanding of the issue here is that while the switch inputs are toggled OFF the Billable label should be the same as the Members must categorize all expenses label (meaning black), since toggled ON are the same currently.

Yeah, we don't fade out the text when the toggle is on, so it should remain black at all times. Nothing changes here from how we do it in the rest of the app 👍

Also, on the Category Settings, I noticed that part of the top screen stays fixed when you scroll which shouldn't be the case:

Keen to hear what rest of @Expensify/design thinks about this too

[Krishna2323]

PROPOSAL UPDATED

• Also included a solution for making the whole view scrollable

[FitseTLT]

Updated according to the above feedback

[dannymcclain]

Nothing changes here from how we do it in the rest of the app 👍

Agree with Jon—the label for the toggle row should be the same color regardless of on/off state. Here's an example from Preferences:

[shawnborton]

Yup, agree with all of those comments.

[ikevin127]

Thanks for the feedback, will review again today!

[ikevin127]

Clarifying timeline of events

• first proposal came in, incomplete as it was not addressing the fix in MoneyRequestView component

• second proposal came in, pretty much a copy of the first proposal (not adding anything new)

• third proposal came in, a complete proposal in the sense that it was addressing the MoneyRequestView component as well, but the issue was that it suggested keeping theme.textSupporting and removing the condition instead:

  To achieve this, we can remove the condition from both places and always apply the theme.textSupporting color.

• I came in as a reviewer and requested design team's input in #53460 (comment)

• design team clarified the expectations, confirming that we don't want the theme.textSupporting (grey / faded) but instead the switch labels should be black (on light theme) and white (on dark theme):

  don't fade out the text when the toggle is on, so it should remain black at all times

• additionally the design team requested a change regarding the top part of the WorkspaceCategoriesSettingsPage which should move with the scroll instead of being fixed (see details in #53460 (comment))

• third proposal was updated, the only change being the additional change requested by the design team related to the top part of the WorkspaceCategoriesSettingsPage moving with the scroll instead of being fixed

• first proposal was updated, adding the missing part related to MoneyRequestView label color:

  we should not fade out on toggle off so we can remove textSupporting styles

  and added a solution for the additional change requested by the design team related to the top part of the WorkspaceCategoriesSettingsPage moving with the scroll instead of being fixed

[!important] The distinction between the first and third proposal is that the first one mentions that we should remove the theme.textSupporting styling completely, while the third proposal mentions removing the condition which would keep the theme.textSupporting styling (grey) which is not what we want according to the design team's feedback.

[ikevin127]

Given the timeline of events detailed in https://github.com/Expensify/App/issues/53460#issuecomment-2530101884, let's go with @FitseTLT's proposal as the root cause of our issue was identified correctly and the solution works as expected given all the requirements, including the additional scrolling requirement requested by design team.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

Offer link Upwork job

[melvin-bot[bot]]

📣 @FitseTLT 🎉 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[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.76-12 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pull/53941

If no regressions arise, payment will be issued on 2024-12-25. :confetti_ball:

For reference, here are some details about the assignees on this issue:

• @FitseTLT requires payment automatic offer (Contributor)
• @ikevin127 requires payment automatic offer (Reviewer)

[melvin-bot[bot]]

@ikevin127 @johncschuster @ikevin127 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[johncschuster]

@ikevin127 and @FitseTLT, I will be OOO starting December 23 and will be returning January 6th. A handful of folks on the BZ team will be online for a few days in between the 25th and the 1st, but we'll be operating with a skeleton crew. I will be issuing my payments when I return on January 6th. If you would like the payment issued sooner, please post this issue in #expensify-open-source and someone on the team will jump in.

Thank you!

[ikevin127]

BugZero Checklist:

• [x] [Contributor] Classify the bug:

Bug classification

Source of bug:

• [ ] 1a. Result of the original design (eg. a case wasn't considered)
• [x] 1b. Mistake during implementation
• [ ] 1c. Backend bug
• [ ] 1z. Other:

Where bug was reported:

• [x] 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• [ ] 2b. Reported on staging (eg. found during regression or PR testing)
• [ ] 2d. Reported on a PR
• [ ] 2z. Other:

Who reported the bug:

• [ ] 3a. Expensify user
• [ ] 3b. Expensify employee
• [ ] 3c. Contributor
• [x] 3d. QA
• [ ] 3z. Other:

• [x] [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: https://github.com/Expensify/App/pull/43749/files#r1891076880 and https://github.com/Expensify/App/pull/27875/files#r1891076896.

• [x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A.

• [x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [x] [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition: The workspace should have billable option enabled -> Enable Rules and Tags in WS > More features > open Tag setting page > press Settings page in Tag settings page > Enable track billable.

1. Open a workspace chat with billable option enabled.
2. Start to create an expense.
3. In confirmation page, verify that the billable label color remains the same when the billable toggle is on and off.
4. Create the expense, tehn open the expense details page.
5. Verify that the billable label color remains the same when the billable toggle is on and off.
6. Open Workspace setting.
7. Choose category.
8. Press on the Settings button on the category setting page.
9. Scroll on the categories setting and verify that the whole page scrolls and the Must categorize all expense top menu doesn't remain fixed when scrolling.

Do we agree 👍 or 👎.

[bfitzexpensify]

Regression test proposed, payments complete.