[$1000] Cursor turns to Arrow on some sections the IOU preview component

[kavimuru]

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

---

Action Performed:

1. Sign in with account A
2. Send an IOU to account B
3. Switch to account B
4. Open the sent IOU
5. Hover over all parts of the IOU preview component

Expected Result:

Cursor shouldn't turn to Arrow since the component has hover effect and the whole preview component should be clickable

Actual Result:

Curosr turns to Arrow on some section of the preview component

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 / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.14-9
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation

2023-05-15.11.01.01.mp4

Recording.617.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1684137959661979

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a8202788bda02ea4
• Upwork Job ID: 1658506650174926848
• Last Price Increase: 2023-05-30

[melvin-bot]

Triggered auto assignment to @conorpendergrast (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[therealsujitk]

Proposal

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

The Pressable is taking the space of the entire action item instead of just the IOUPreview component.

What is the root cause of that problem?

This is default behaviour of react native, components (In this case Pressable) expand to fill it's parent as react native follows a flexbox system to style components.

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

To resolve this, we simply apply the IOUPreview component styles to the Pressable instead of the View. This code has to be replaced with the code below,

const child = (
    <>
        <View style={[styles.flexRow]}>
            <View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
                <Text style={[styles.textLabelSupporting, styles.lh16]}>{props.isBillSplit ? props.translate('iou.split') : props.translate('iou.cash')}</Text>
                {Boolean(getSettledMessage()) && (
                    <>
                        <Icon
                            src={Expensicons.DotIndicator}
                            width={4}
                            height={4}
                            additionalStyles={[styles.mr1, styles.ml1]}
                        />
                        <Text style={[styles.textLabelSupporting, styles.lh16]}>{getSettledMessage()}</Text>
                    </>
                )}
            </View>
        </View>
        <View style={[styles.flexRow]}>
            <View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
                <Text style={styles.textHeadline}>{CurrencyUtils.convertToDisplayString(requestAmount, requestCurrency)}</Text>
                {!props.iouReport.hasOutstandingIOU && !props.isBillSplit && (
                    <View style={styles.iouPreviewBoxCheckmark}>
                        <Icon
                            src={Expensicons.Checkmark}
                            fill={themeColors.iconSuccessFill}
                        />
                    </View>
                )}
            </View>
            {props.isBillSplit && (
                <View style={styles.iouPreviewBoxAvatar}>
                    <MultipleAvatars
                        icons={participantAvatars}
                        secondAvatarStyle={[styles.secondAvatarInline, props.isHovered ? styles.iouPreviewBoxAvatarHover : undefined]}
                        avatarTooltips={participantEmails}
                    />
                </View>
            )}
        </View>

        <View style={[styles.flexRow]}>
            <View style={[styles.flex1]}>
                {!isCurrentUserManager && props.shouldShowPendingConversionMessage && (
                    <Text style={[styles.textLabel, styles.colorMuted]}>{props.translate('iou.pendingConversionMessage')}</Text>
                )}
                {!_.isEmpty(requestComment) && <Text style={[styles.colorMuted]}>{requestComment}</Text>}
            </View>
            {props.isBillSplit && !_.isEmpty(participantEmails) && (
                <Text style={[styles.textLabel, styles.colorMuted, styles.ml1]}>
                    {props.translate('iou.amountEach', {
                        amount: CurrencyUtils.convertToDisplayString(IOUUtils.calculateAmount(participantEmails.length - 1, requestAmount), requestCurrency),
                    })}
                </Text>
            )}
        </View>
    </>
);

let childContainer;
if (props.onPreviewPressed) {
    childContainer = (
        <Pressable
            style={[styles.iouPreviewBox, ...props.containerStyles]}
            onPress={props.onPreviewPressed}
            onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
            onPressOut={() => ControlSelection.unblock()}
            onLongPress={showContextMenu}
        >
            {child}
        </Pressable>
    );
} else {
    childContainer = <View style={[styles.iouPreviewBox, ...props.containerStyles]}>{child}</View>;
}

return (
    <OfflineWithFeedback
        pendingAction={props.pendingAction}
        errors={props.walletTerms.errors}
        onClose={() => {
            PaymentMethods.clearWalletTermsError();
            Report.clearIOUError(props.chatReportID);
        }}
        errorRowStyles={[styles.mbn1]}
        needsOffscreenAlphaCompositing
    >
        {childContainer}
    </OfflineWithFeedback>
);

Note: We move the OfflineWithFeedback component outside the Pressable because we don't want it to get cut or the Presseble events to interfere with the ones in OfflineWithFeedback.

What alternative solutions did you explore? (Optional)

None.

[conorpendergrast]

Reproduced now, thanks!

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[conorpendergrast]

@rushatgabhane there's a speedy proposal here, if you could give that a review!

[conorpendergrast]

@rushatgabhane Bump on reviewing that proposal, please

[melvin-bot]

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

[papistacoding]

Hey, I have a solution to this issue. I submitted a proposal to upwork.
Regards,
Bruno

[melvin-bot]

📣 @papistacoding! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[papistacoding]

Hey, I have solution to this issue.
Upwork proposal is submitted
Regards,
Bruno

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~0154ea82382acb8539

[conorpendergrast]

@papistacoding Hey! We only accept proposals on the github issue (right where you are!), the format you see above please. Thanks!

[papistacoding]

@conorpendergrast Thanks for info. So proposal from @therealsujitk has been already approved?

[conorpendergrast]

Not so far! So there's still an opportunity to post a fresh proposal here in that format, that @rushatgabhane can then review

[papistacoding]

@conorpendergrast sorry to ask so many questions. I got the resolution working as intended, but how do I earn money on upwork? First I put proposal here? please tag me in reply so I get a notification

[conorpendergrast]

@papistacoding I'd recommend taking a look at this guide, and then asking any questions in the #expensify-open-source Slack channel!

[papistacoding]

Proposal

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

The Pressable is taking the space of the entire action item instead of just the IOUPreview component.

What is the root cause of that problem?

This is default behaviour of react native, components (In this case Pressable) expand to fill it's parent as react native follows a flexbox system to style components.

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

To resolve this, we add max-width to pressable component and we add the margin of mt2. we remove margin from inner element which comes from props. we remove unnecessary props.

1. adding maxWidth and margin ( IOUPreview.js component):

return (
      <Pressable
          onPress={props.onPreviewPressed}
          onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
          onPressOut={() => ControlSelection.unblock()}
          onLongPress={showContextMenu}
          style={{maxWidth: variables.sideBarWidth, ...styles.mt2}}
      >
          {childContainer}
      </Pressable>
  );

2. removing props from MoneyRequestAction.js:

const propTypes = {
    /** All the data of the action */
    action: PropTypes.shape(reportActionPropTypes).isRequired,

    /** The ID of the associated chatReport */
    chatReportID: PropTypes.string.isRequired,

    /** The ID of the associated request report */
    requestReportID: PropTypes.string.isRequired,

    /** Is this IOUACTION the most recent? */
    isMostRecentIOUReportAction: PropTypes.bool.isRequired,

    /** Popover context menu anchor, used for showing context menu */
    contextMenuAnchor: refPropTypes,

    /** Callback for updating context menu active state, used for showing context menu */
    checkIfContextMenuActive: PropTypes.func,

    /* Onyx Props */
    /** chatReport associated with iouReport */
    chatReport: PropTypes.shape({
        /** The participants of this report */
        participants: PropTypes.arrayOf(PropTypes.string),

        /** Whether the chat report has an outstanding IOU */
        hasOutstandingIOU: PropTypes.bool.isRequired,
    }),

    /** IOU report data object */
    iouReport: iouReportPropTypes,

    /** Array of report actions for this report */
    reportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)),

    /** Whether the IOU is hovered so we can modify its style */
    isHovered: PropTypes.bool,

    network: networkPropTypes.isRequired,

    /** Session info for the currently logged in user. */
    session: PropTypes.shape({
        /** Currently logged in user email */
        email: PropTypes.string,
    }),

    ...withLocalizePropTypes,
};

const defaultProps = {
    contextMenuAnchor: undefined,
    checkIfContextMenuActive: () => {},
    chatReport: {
        participants: [],
    },
    iouReport: {},
    reportActions: {},
    isHovered: false,
    session: {
        email: null,
    },
};

return (
       <>
           <IOUPreview
               iouReportID={props.requestReportID}
               chatReportID={props.chatReportID}
               isBillSplit={hasMultipleParticipants}
               action={props.action}
               contextMenuAnchor={props.contextMenuAnchor}
               checkIfContextMenuActive={props.checkIfContextMenuActive}
               shouldShowPendingConversionMessage={shouldShowPendingConversionMessage}
               onPreviewPressed={onIOUPreviewPressed}
               containerStyles={[styles.cursorPointer, props.isHovered ? styles.iouPreviewBoxHover : undefined]}
               isHovered={props.isHovered}
           />
       </>
   );

this resolves the issue completely and we keep the structure of IOUPreview untouched
EDIT: since Pressable is now parent element with max-width set, we can remove max width in styles.iouPreviewBox:

 iouPreviewBox: {
        backgroundColor: themeColors.cardBG,
        borderRadius: variables.componentBorderRadiusCard,
        padding: 20,
        width: '100%',
    }

[therealsujitk]

Hey @papistacoding 👋 you'll have to properly format your proposal using Markdown and your code should go in code blocks with syntax highlighting otherwise your proposal just becomes unreadable and reviewers will probably just ignore it.

The proposal's markdown code can be obtained here.

[papistacoding]

@therealsujitk thanks a lot for helping me out. I rewrote the entire coment

[conorpendergrast]

@rushatgabhane Two proposals for your to review here!

[therealsujitk]

I found another issue with the same RCA - #19717. I think both of these can be fixed together.

[hoangzinh]

Just wanna verify the expectation. We would like to turn the cursor to pointer when it hovers in

• (1) IOU preview
• or (2) whole reportAction

[therealsujitk]

@hoangzinh Now that you mention it, I re-read the issue and I might have been wrong about the expected result. However personally I think we should change it to 1. There were several cases where I clicked on 2 just to switch to the browser window (if another window was open on top) or to lose focus from the composer and it took me to the IOU report which really annoyed me.

@conorpendergrast @yuwenmemon @rushatgabhane can you confirm what the expected result should be?

[rushatgabhane]

or (2) whole reportAction

for 2 @hoangzinh

[hoangzinh]

@Natnael-Guchima could share how to make a report preview like your recording?

On my end, I can either show the IOU preview (Cash + amount) or the green CTA "Pay". I can't show both of them in 1 report preview.

[Natnael-Guchima]

@hoangzinh sorry for my late reply. I will take a look at this tomorrow and I will late you know.

[Natnael-Guchima]

@hoangzinh Yes, it doesn't seem you can show the pay button and the IOU preview components together - they seem to be separated on the latest releases. The cursor issue persists on both of the separated components though.

2023-05-29.22.30.23.mp4

[greg-schroeder]

@rushatgabhane @yuwenmemon - I have this related issue: #19717 - do you think that should be folded into this one or handled separately?

[therealsujitk]

@greg-schroeder can you confirm the expected result as mentioned in this comment as well (The OP seems to say 2 is the expected result but I just want to be sure), depending on that the two issues may or may not be related and I may have to rewrite my proposal.

[hoangzinh]

@greg-schroeder the current expected behavior is different between two issues. Also it's different reportAction type so I think we can handle them separately.

[melvin-bot]

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

[greg-schroeder]

I don't know which approach is correct, so I'll wait on @yuwenmemon or @rushatgabhane to weigh in

[rushatgabhane]

I have this related issue: #19717 - do you think that should be folded into this one or handled separately?

@greg-schroeder i think both issues have different expectations.
Entire IOU should be clickable (even when clicked outside), and the attachment shouldn't be downloaded by clicking outside. #19717 (comment)

[rushatgabhane]

We should keep them as separate issues

[rushatgabhane]

@therealsujitk @papistacoding I'm sorry I don't understand the proposals.
Could you please update them by explaining it in plain english without any code diffs.

Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text (src)

[rushatgabhane]

@yuwenmemon @conorpendergrast I vote that we do nothing.

Cursor turns to hand ✋ only in this red section, and it's clickable only in the red section.
I'd like to see a feature request with a problem solution statement. This is not a bug.

[therealsujitk]

Cursor turns to hand only in this red section, and it's clickable only in the red section.

I believe this is already the case (though I personally don't like this behaviour), can you check with staging and confirm?

Currently my proposal changes it so only the light green portion is clickable.

[melvin-bot]

@yuwenmemon @conorpendergrast @rushatgabhane 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!

[conorpendergrast]

@yuwenmemon What's your take here? I'm erring on agreeing with @therealsujitk that this should actually be a feature request, not a bug

[yuwenmemon]

Agreed! 👍

[conorpendergrast]

Ok, closing as a feature request, not a bug!