[$500] Web - Assign task - Tool tip starts to be inverted after clicking on edit and making a post unread

[izarutskaya]

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. Assign a task to account B
2. While inside the task thread send a message
3. Hover over the avatar and the name and observe the position of the tooltip
4. Hover over the last sent message and click on the edit icon
5. Go to the LHN and right click on the open chat and click on Mark as unread
6. Go to the edit box and click on the close icon
7. Hover over the avatar and on the account name again and observe the position of the tooltip

Expected Result:

The position of the tooltip should not change

Actual Result:

The position of the tooltip inverts

Workaround:

Unknown

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: v1.3.62-4

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-08-25.10.06.12.mp4

Recording.1424.mp4

Expensify/Expensify Issue URL:

Issue reported by: @Nathan-Mulugeta

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692947480477819

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01acf203d863453a81
• Upwork Job ID: 1698691177926508544
• Last Price Increase: 2023-09-18

[melvin-bot]

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

[trjExpensify]

We've now got multiple issues with the tooltip being "inverted" in certain cases:

#26041
#22222

I think someone should tackle them all, as I suspect there's a common RCA.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[MuhammadSaboorIslam]

I also propose @WasilIslam solution

[melvin-bot]

📣 @MuhammadSaboorIslam! 📣
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>

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[rmm-fl]

For reproduction, just navigating away and reopening the room/report is enough:

repro-26650.mp4

[rmm-fl]

Proposal

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

Tool tip position is inverted after making a post unread and reopening the room/report

What is the root cause of that problem?

The unread marker which is absolute positioned to show above the message, interferes with the tool-tip position calculation here (verified by returning false):

App/src/styles/getTooltipStyles.js

Lines 91 to 96 in c805b67

	// Ensure it's not overlapping with another element by checking if the yOffset is greater than the top of the element
	// and less than the bottom of the element. Also ensure the tooltip target is not completely inside the elementAtTargetCenterX by vertical direction
	const isOverlappingAtTargetCenterX = yOffset > rectAtTargetCenterX.top && yOffset < rectAtTargetCenterX.bottom && yOffset + tooltipTargetHeight > rectAtTargetCenterX.bottom;
	return isOverlappingAtTargetCenterX;
	}

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

Change the style of UnreadActionIndicator removing the absolute positioning which will result in the green line to be displayed between the messages, instead of it being displayed like a border at the top of the message container. Doing so will no longer interfere with the tooltip's calculations, thus fixing the issue.

Before.mp4

After.mp4

Doing this will also match the design with other horizontal lines already present, for example:

What alternative solutions did you explore? (Optional)

--

Result

Here's a proof that it fixes the issue:

result.mp4

[rmm-fl]

Proposal 2

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

Tool tip position is inverted after making a post unread and reopening the room/report

What is the root cause of that problem?

The unread marker is absolute positioned to the top of post, thus overlapping the post near the top. This results in tooltip's overlapping detection to kick in here:

App/src/styles/getTooltipStyles.js

Lines 73 to 80 in 1bdc5ac

	function isOverlappingAtTop(xOffset, yOffset, tooltip, tooltipTargetWidth, tooltipTargetHeight) {
	if (typeof document.elementFromPoint !== 'function') {
	return false;
	}
	// Use the x center position of the target to prevent wrong element returned by elementFromPoint
	// in case the target has a border radius or is a multiline text.
	const targetCenterX = xOffset + tooltipTargetWidth / 2;

And because of that it decides to show the tooltip below the container, here:

App/src/styles/getTooltipStyles.js

Lines 156 to 160 in 1bdc5ac

	// Determine if the tooltip should display below the wrapped component.
	// If either a tooltip will try to render within GUTTER_WIDTH logical pixels of the top of the screen,
	// Or the wrapped component is overlapping at top-center with another element
	// we'll display it beneath its wrapped component rather than above it as usual.
	shouldShowBelow = yOffset - tooltipHeight < GUTTER_WIDTH || isOverlappingAtTop(xOffset, yOffset, tooltip, tooltipTargetWidth, tooltipTargetHeight);

(The reason it only triggers when opening and closing the report is because, tooltips are memoized, and it doesn't refresh when clicking the unread button on the post.)

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

We should add an exemption for the "unread indicator" to allow the overlapping of tooltip on top of the "unread indicator". In similar way as we do for SelectionScraper.js by adding a new attribute.

Update the UnreadActionIndicator indicator component to add the attribute tooltip-allow-overlap (or a better name):

// src/components/UnreadActionIndicator.js
function UnreadActionIndicator(props) {
    return (
        <View
            accessibilityLabel={props.translate('accessibilityHints.newMessageLineIndicator')}
            data-action-id={props.reportActionID}
            style={[styles.unreadIndicatorContainer, styles.userSelectNone]}
            dataSet={{
                [CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true,
                ['tooltip-allow-overlap']: true, // TODO: move to CONST in PR
            }}
        >
...

And update the isOverlappingAtTop function to ignore if element contains that attribute:

// src/styles/getTooltipStyles.js
...
    // Some elements that are absolute positioned at the top of the content, can be ignored if
    // such element explicitly adds the 'tooltip-allow-overlap' data attribute.
    // For example, the unread indicator is _OK_ to ignore because it is really thin.
    if (isOverlappingAtTargetCenterX && elementAtTargetCenterX.hasAttribute('data-tooltip-allow-overlap')) {
        return false;
    }

    return isOverlappingAtTargetCenterX;
}

Result

result.mp4

What alternative solutions did you explore? (Optional)

--

[rmm-fl]

@situchan can you please take a look at my proposal, Thanks!

[trjExpensify]

@situchan can you review this one please? Thanks!

[trjExpensify]

Bump, @situchan!

[melvin-bot]

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

[melvin-bot]

@trjExpensify, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@trjExpensify @situchan 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!

[situchan]

reviewing today

[melvin-bot]

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

[trjExpensify]

@situchan did you review?

[situchan]

I tested against latest main but not reproducible to me.

Screen.Recording.2023-09-20.at.5.36.15.PM.mov

@Nathan-Mulugeta are you still able to reproduce?

[situchan]

@rmm-fl I followed your repro step in #26650 (comment) but still not reproducible. Can you?

[rmm-fl]

@situchan Same as you, I can't reproduce it anymore on latest main.

[situchan]

It would be great if you can find PR which fixed this.

[Nathan-Mulugeta]

I can't reproduce this anymore as well

[situchan]

@Nathan-Mulugeta are you able to find the PR so you can claim bug reporting bonus?

[trjExpensify]

Cool, I think we close this out then. @Nathan-Mulugeta if you find the PR, feel free to let me know here and I'll reopen!