[HOLD for payment 2023-11-06] [$500] Dev - wider gap between mention or emoji suggestion box and composer

[kavimuru]

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

---

Action Performed:

1. Open a chat
2. Type ':' & start searching for emoji
3. Observe the gap between the suggestion box and the composer
4. Clear the composer & type '@'
5. Observe the gap between the suggestion box and the composer

Expected Result:

The margin between the suggestion box and the composer should be smaller

Actual Result:

The margin between the suggestion box and the composer is wider compared to the gap it has on staging

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:
Reproducible in staging?: dev
Reproducible in production?: dev
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

Screencast.from.2023-08-25.22-09-58.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011eeb44265c91fc05
• Upwork Job ID: 1697231915838853120
• Last Price Increase: 2023-09-21
• Automatic offers:
• situchan | Contributor | 26876537
• natnael-guchima | Reporter | 26876542
• Pujan92 | Contributor | 27301559

[melvin-bot]

Triggered auto assignment to @greg-schroeder (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

[melvin-bot]

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

[melvin-bot]

Current assignee @greg-schroeder is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[greg-schroeder]

This is a regression but we're handling it as a normal External issue per team discussion in linked thread

[Pujan92]

Proposal

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

Extra local time space between composer and suggestions

What is the root cause of that problem?

We are using a parent View ref for measureContainer which contains ParticipantLocalTime, with that suggestions list starting on top of the parent View.

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Lines 318 to 320 in 0477b33

	<View
	ref={containerRef}
	style={[shouldShowReportRecipientLocalTime && !lodashGet(network, 'isOffline') && styles.chatItemComposeWithFirstRow, isComposerFullSize && styles.chatItemFullComposeRow]}

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

Instead of setting the prop ref on the parent View, apply it to the wrapper View of the ComposerWithSuggestions here.

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Lines 329 to 334 in 0477b33

	<View
	style={[
	shouldUseFocusedColor ? styles.chatItemComposeBoxFocusedColor : styles.chatItemComposeBoxColor,
	styles.flexRow,
	styles.chatItemComposeBox,
	isComposerFullSize && styles.chatItemFullComposeBox,

To make it work on native we need to create new OfflineWithFeedback which only has ParticipantLocalTime part as a child and create a new View for the rest content. Plan is to move PortalHost nearer to the Composer view and separate it from the ParticipantLocalTime view.

So structure will be

Main View Start

• OfflineWithFeedback -> ParticipantLocalTime
• View -> PortalHost -> OfflineWithFeedback -> rest content

Main View Ends

Snippet

return (
        <View
            style={[shouldShowReportRecipientLocalTime && !lodashGet(network, 'isOffline') && styles.chatItemComposeWithFirstRow, isComposerFullSize && styles.chatItemFullComposeRow]}
        >
            <OfflineWithFeedback>
                {shouldShowReportRecipientLocalTime && hasReportRecipient && <ParticipantLocalTime participant={reportRecipient} />}
            </OfflineWithFeedback>
            <View>
                <PortalHost name="suggestions" />
            
                <OfflineWithFeedback
                    pendingAction={pendingAction}
                    style={isComposerFullSize ? styles.chatItemFullComposeRow : {}}
                    contentContainerStyle={isComposerFullSize ? styles.flex1 : {}}
                >
                    <View
                        ref={containerRef}
                        style={[
                            shouldUseFocusedColor ? styles.chatItemComposeBoxFocusedColor : styles.chatItemComposeBoxColor,
                            styles.flexRow,
                            styles.chatItemComposeBox,
                            isComposerFullSize && styles.chatItemFullComposeBox,
                            hasExceededMaxCommentLength && styles.borderColorDanger,
                        ]}
                    >

Result

Screen.Recording.2023-08-31.at.19.50.50.mp4

Old

## Proposal

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

Extra local time space between composer and suggestions

What is the root cause of that problem?

We are providing a prop called shouldShowReportRecipientLocalTime from the ReportActionCompose which will consider the shouldIncludeReportRecipientLocalTimeHeight in the AutoSuggestions and that leads to giving that padding of that height which will decrease the top value and cause the issue.

Reason: Earlier we had suggestions not directly connected to the Composer but with the View which shows the ParticipantLocalTime and Composer, so we needed to check shouldIncludeReportRecipientLocalTimeHeight to show the suggestions that reduce the height of the ParticipantLocalTime in the padding.
But now these are connected to the Composer and separated from the ParticipantLocalTime which causes the extra padding issue.

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Lines 336 to 374 in 24d820a

	{shouldShowReportRecipientLocalTime && hasReportRecipient && <ParticipantLocalTime participant={reportRecipient} />}
	<View
	style={[
	shouldUseFocusedColor ? styles.chatItemComposeBoxFocusedColor : styles.chatItemComposeBoxColor,
	styles.flexRow,
	styles.chatItemComposeBox,
	isComposerFullSize && styles.chatItemFullComposeBox,
	hasExceededMaxCommentLength && styles.borderColorDanger,
	]}
	>
	<AttachmentModal
	headerTitle={translate('reportActionCompose.sendAttachment')}
	onConfirm={addAttachment}
	onModalShow={() => setIsAttachmentPreviewActive(true)}
	onModalHide={onAttachmentPreviewClose}
	>
	{({displayFileInModal}) => (
	<>
	<AttachmentPickerWithMenuItems
	displayFileInModal={displayFileInModal}
	reportID={reportID}
	report={report}
	reportParticipantIDs={reportParticipantIDs}
	isFullComposerAvailable={isFullComposerAvailable}
	isComposerFullSize={isComposerFullSize}
	updateShouldShowSuggestionMenuToFalse={updateShouldShowSuggestionMenuToFalse}
	isBlockedFromConcierge={isBlockedFromConcierge}
	disabled={disabled}
	setMenuVisibility={setMenuVisibility}
	isMenuVisible={isMenuVisible}
	onTriggerAttachmentPicker={onTriggerAttachmentPicker}
	onCanceledAttachmentPicker={restoreKeyboardState}
	onMenuClosed={restoreKeyboardState}
	onAddActionPressed={onAddActionPressed}
	actionButtonRef={actionButtonRef}
	/>
	<ComposerWithSuggestions
	ref={composerRef}
	animatedRef={animatedRef}

https://github.com/Expensify/App/pull/25758/files#diff-7fa482b2895f4e36a2a3ad8233496489c6de1256a8c992524e2fc072aaba43f8L1051-L1058

App/src/styles/StyleUtils.js

Line 1074 in dd71b38

const optionalPadding = shouldIncludeReportRecipientLocalTimeHeight ? CONST.RECIPIENT_LOCAL_TIME_HEIGHT : 0;

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

We don't need to consider the shouldShowReportRecipientLocalTime value bcoz now Suggestions and Composer are in a View that doesn't contain the ParticipantLocalTime, so we don't require to adjust the padding considering whatever ParticipantLocalTime height. With that, we can get rid of the shouldShowReportRecipientLocalTime prop and shouldIncludeReportRecipientLocalTimeHeight in AutoCompleteSuggestions as it seems not required now.

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.js

Line 392 in dd71b38

shouldShowReportRecipientLocalTime={shouldShowReportRecipientLocalTime}

Result

Screen.Recording.2023-08-31.at.19.50.50.mp4

[melvin-bot]

@greg-schroeder, @abdulrahuman5196 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[greg-schroeder]

Awaiting proposal review

[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]

bump @abdulrahuman5196

[melvin-bot]

@greg-schroeder, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@greg-schroeder, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[greg-schroeder]

bump @abdulrahuman5196

[melvin-bot]

@greg-schroeder @abdulrahuman5196 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!

[melvin-bot]

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

[melvin-bot]

📣 @Pujan92 🎉 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 📖

[greg-schroeder]

Not overdue, awaiting PR to be opened

[Pujan92]

@situchan PR is ready for review!

[melvin-bot]

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

[melvin-bot]

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

• Fix: wider gap between mention or emoji suggestion box and composer #30226

If no regressions arise, payment will be issued on 2023-11-06. 🎊

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

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

• @Pujan92 requires payment offer (Contributor)
• @situchan requires payment offer (Contributor)
• @Natnael-Guchima requires payment offer (Reporter)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@situchan] The PR that introduced the bug has been identified. Link to the PR:
• [@situchan] 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:
• [@situchan] A discussion in #expensify-bugs 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:
• [@situchan] Determine if we should create a regression test for this bug.
• [@situchan] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[greg-schroeder]

Working on payments now

[situchan]

• The PR that introduced the bug has been identified. Link to the PR: Performance: chat input #25758
• 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: Performance: chat input #25758 (comment)
• A discussion in #expensify-bugs 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
• Determine if we should create a regression test for this bug.
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

This is minor style issue and caused by big composer refactor, I don't see the need of new regression test. Users can still chat, mention users and send emoijs.

[greg-schroeder]

Payments made now, not filing regression test per above