[HOLD for payment 2023-06-21] [$1000] Inconsistent styling on create task push to page

[thienlnam]

https://expensify.slack.com/archives/C04QEB4MJEQ/p1685378425623999

The labels should use our textSupporting color The small label should be closer to the value the arrows/carets should use our icon color and should be at 20x20

The style on the second step of creating a task is inconsistent with the push-to-page style from other places in the app. I feel like we keep recreating this style instead of just using the same global component.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018dddd8a08831da39
• Upwork Job ID: 1664377958553657344
• Last Price Increase: 2023-06-01

[thienlnam]

Current:

Styles should look like

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to @muttmuure (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Current assignee @thienlnam is eligible for the External assigner, not assigning anyone new.

[allroundexperts]

Proposal

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

Make task page confirmation styles consistent

What is the root cause of that problem?

Not a bug but a feature.

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

We have to edit our MenuItem component such that it supports following new props:

1. withoutHover: By default, MenuItem when hovered, shows a different background colour. To circumvent this, we have to check the value of the new props withoutHover here. This gets replaced by:

   StyleUtils.getButtonBackgroundColorStyle(getButtonState(props.focused || (props.withHover && hovered), pressed, props.success, props.disabled, props.interactive), true),

2. alternateText: In order to display text on top of the MenuItem like in the image below, we have to use another prop called alternateText. This can be added here:

   {props.alternateText &&
               <View style={[styles.pl5]}>
                   <Text style={StyleUtils.combineStyles(styles.sidebarLinkText, styles.optionAlternateText, styles.textLabelSupporting, styles.pre)}>{props.alternateText}</Text>
               </View>
           }

   

Lastly, we also need to support an array of icon in the MenuItems component. In order to achieve this, we have to change this condition to:

    {Boolean(props.icon) && !_.isArray(props.icon) && (

At top of this condition, we have to add the following:

{Boolean(props.icon) && _.isArray(props.icon) && (
      <MultipleAvatars
          icons={props.icon}
          size={CONST.AVATAR_SIZE.DEFAULT}
          secondAvatarStyle={[StyleUtils.getBackgroundAndBorderStyle(themeColors.appBG)]}
      />
)}

Once these changes are applied, we can use MenuItem instead of TaskSelectorLink as follow:

<MenuItem
          alternateText={assignee.displayName ? props.translate('newTaskPage.assignee'): ''}
          title={assignee.displayName ? assignee.displayName : props.translate('newTaskPage.assignee')}
          description={assignee.subtitle}
          onPress={() => Navigation.navigate(ROUTES.NEW_TASK_ASSIGNEE)}
          shouldShowRightIcon
          icon={assignee.icons}
          withHover={false}
/>

Once we use the MenuItem we won't need the horizontal padding that we use here.

The approach can be further optimised in a PR.

What alternative solutions did you explore? (Optional)

This needs to be changed to:

   <Text style={[styles.label, styles.textLabelSupporting, props.icons.length > 0 ? styles.mb2 : styles.mb0]}>{props.translate(props.label)}</Text>

This needs to be changed to:

<Icon
    src={Expensicons.ArrowRight}
    fill={themeColors.icon}
    width={variables.iconSizeNormal}
    height={variables.iconSizeNormal}
    inline
/>

Result

[sethraj14]

Proposal

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

Inconsistent styling on create task push to page

What is the root cause of that problem?

Not providing right styles to the text and icons

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

We could make the styles consistent by changing these lines -

<Text style={[styles.label, styles.textLabelSupporting, props.icons.length > 0 ? styles.mb2 : styles.mb0]}>{props.translate(props.label)}</Text>

and pass these styles to icon -

<Icon src={Expensicons.ArrowRight} fill={themeColors.icon} width={variables.iconSizeNormal} height={variables.iconSizeNormal} inline />

What alternative solutions did you explore? (Optional)

[s77rt]

@allroundexperts Thanks for the proposal. Can we re-use MenuItemWithTopDescription instead of recreating the component?

[s77rt]

@sethraj14 Thanks for the proposal. Same question as above ^

[allroundexperts]

@allroundexperts Thanks for the proposal. Can we re-use MenuItemWithTopDescription instead of recreating the component?

Yep. We can. Posting 'how' in some time.

[sethraj14]

@allroundexperts Thanks for the proposal. Can we re-use MenuItemWithTopDescription instead of recreating the component?

Yes, I was exploring the same as we use this in every other place.

[allroundexperts]

@s77rt Proposal updated.

[sethraj14]

@s77rt

Proposal

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

Inconsistent styling on create task push to page

What is the root cause of that problem?

Not providing right styles to the text and icons

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

In order to shift to MenuItemWithTopDescription we have these fields to map -

1. label -> description (with props.translate())
2. text -> title
3. onPress -> onPress
4. disabled -> shouldShowRightIcon
5. alternateText -> Add new props alternateText to MenuItem
6. icon -> Add support for array of icons

After adding these props, we need to change the MenuItem file like this here-

                                {Boolean(props.icon) && _.isArray(props.icon) && (
                                    <MultipleAvatars
                                        icons={props.icon}
                                        size={CONST.AVATAR_SIZE.DEFAULT}
                                        secondAvatarStyle={[StyleUtils.getBackgroundAndBorderStyle(themeColors.appBG)]}
                                    />
                                )}
                                {Boolean(props.alternateText) && (
                                    <View style={[styles.pl0]}>
                                        <Text
                                            style={titleTextStyle}
                                            numberOfLines={1}
                                        >
                                            {convertToLTR(props.title)}
                                        </Text>
                                        <Text style={StyleUtils.combineStyles(styles.sidebarLinkText, styles.optionAlternateText, styles.textLabelSupporting, styles.pre)}>
                                            {props.alternateText}
                                        </Text>
                                    </View>
                                )}

Hover should be there as it will be consistent with other similar screens.

Discard isShareDestination as it is not doing anything in the current implementation

Finally the NewTaskPage file will look like this -

<ScrollView>
                <View style={styles.flex1}>
                    <MenuItemWithTopDescription
                        description={props.translate('newTaskPage.title')}
                        title={props.task.title}
                        onPress={() => Navigation.navigate(ROUTES.NEW_TASK_TITLE)}
                        shouldShowRightIcon={true}
                    />
                    <MenuItemWithTopDescription
                        description={props.translate('newTaskPage.description')}
                        title={props.task.description}
                        onPress={() => Navigation.navigate(ROUTES.NEW_TASK_DESCRIPTION)}
                        shouldShowRightIcon={true}
                    />
                    <MenuItemWithTopDescription
                        alternateText={assignee.subtitle}
                        description={props.translate('newTaskPage.assignee')}
                        title={assignee.displayName || ''}
                        onPress={() => Navigation.navigate(ROUTES.NEW_TASK_ASSIGNEE)}
                        shouldShowRightIcon={true}
                        icon={assignee.icons}
                    />
                    <MenuItemWithTopDescription
                        alternateText={shareDestination.subtitle}
                        description={props.translate('newTaskPage.shareSomewhere')}
                        title={shareDestination.displayName || ''}
                        onPress={() => Navigation.navigate(ROUTES.NEW_TASK_SHARE_DESTINATION)}
                        shouldShowRightIcon={true}
                        icon={shareDestination.icons}
                    />
                    <FormAlertWithSubmitButton
                        isAlertVisible={submitError}
                        message={errorMessage}
                        onSubmit={() => onSubmit()}
                        enabledWhenOffline
                        buttonText={props.translate('newTaskPage.confirmTask')}
                        containerStyles={[styles.mh0, styles.mt5, styles.flex1]}
                    />
                </View>
            </ScrollView>

The final solution will be cleaned and optimized.

What alternative solutions did you explore? (Optional)

We could make the styles consistent by changing these lines -

<Text style={[styles.label, styles.textLabelSupporting, props.icons.length > 0 ? styles.mb2 : styles.mb0]}>{props.translate(props.label)}</Text>

and pass these styles to icon -

<Icon src={Expensicons.ArrowRight} fill={themeColors.icon} width={variables.iconSizeNormal} height={variables.iconSizeNormal} inline />

[melvin-bot[bot]]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[s77rt]

@allroundexperts Thanks. This is looking good but let's not worry about the hover style, it's better to keep it for consistency.

:ribbon: :eyes: :ribbon: C+ reviewed

cc @thienlnam

[s77rt]

@sethraj14 Thanks. I think the idea is the same as @allroundexperts's so we will just go with the first. But feel free to check other issues with Help Wanted label here.

[melvin-bot[bot]]

📣 @allroundexperts You have been assigned to this job by @thienlnam! Please apply to this job in Upwork 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 📖

[thienlnam]

Assigned, thanks! Also one more thing can we make sure to add a bold style to the assignee name while we're already here?

[allroundexperts]

PR created https://github.com/Expensify/App/pull/20145

[melvin-bot[bot]]

@s77rt, @allroundexperts, @thienlnam, @muttmuure Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[s77rt]

Not overdue. PR is merged.

[muttmuure]

Not on prod yet

[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 1.3.27-7 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/20145

If no regressions arise, payment will be issued on 2023-06-21. :confetti_ball:

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• [ ] External issue reporter
• [ ] Contributor that fixed the issue
• [ ] Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[muttmuure]

@allroundexperts @s77rt invited to job

[s77rt]

@muttmuure Applied!

[muttmuure]

Offer sent to @s77rt. Still waiting on @allroundexperts to apply

[allroundexperts]

Offer sent to @s77rt. Still waiting on @allroundexperts to apply

Can you send me the link please?

[s77rt]

@muttmuure Accepted!

[muttmuure]

@s77rt paid, @allroundexperts offer sent

[allroundexperts]

@s77rt paid, @allroundexperts offer sent

@muttmuure Accepted!

[thienlnam]

I think we're all good here? @allroundexperts Did your contract get paid out?

[allroundexperts]

Seems like its still in progress @thienlnam.

[muttmuure]

Contract paid out for @allroundexperts too.

Thanks for your patience here!