Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD https://github.com/Expensify/App/pull/20512 $1000] Web - New tooltip is not displayed over workspace direct chats and general workspace chat name #20716

Closed
1 of 6 tasks
lanitochka17 opened this issue Jun 13, 2023 · 33 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 13, 2023

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


Issue found when executing PR #20276

Action Performed:

  1. Login to NewDot with account with workspace with several members
  2. Hover over workspace direct chat user names and icons (and over general workspace chat)

Expected Result:

New tooltip should be displayed

Actual Result:

Tooltip is missing

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: 1.3.27.4

Reproducible in staging?: Yes

Reproducible in production?: No

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

Bug6091706_video_55__2_.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01443ce644ff2fc4f2
  • Upwork Job ID: 1671866330621509632
  • Last Price Increase: 2023-06-22
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment Engineering labels Jun 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

Triggered auto assignment to @iwiznia (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@isagoico
Copy link

Issue found while executing PR #20276

@iwiznia
Copy link
Contributor

iwiznia commented Jun 13, 2023

@puneetlath @fedirjh is this something we missed on #20276 or is this how it is supposed to work? I assume the former...

@iwiznia iwiznia removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Jun 13, 2023
@iwiznia iwiznia removed their assignment Jun 13, 2023
@iwiznia iwiznia added the Bug Something is broken. Auto assigns a BugZero manager. label Jun 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Jun 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

@fedirjh
Copy link
Contributor

fedirjh commented Jun 13, 2023

is this something we missed

@iwiznia I think that when we hover over a user's avatar and email, it should display a new tooltip. However, I am not aware of any specific details regarding the workspace itself.

@iwiznia
Copy link
Contributor

iwiznia commented Jun 13, 2023

Yeah, I don't see any mention in the OG issue of other entries in the LHN that are not DMs... but our LHN has a lot of things that are not DMs. So the question is: do we want to show a tooltip for all the other cases or not? I'd assume we do want to do that, though not sure what the tooltip would show.

@melvin-bot melvin-bot bot added the Overdue label Jun 15, 2023
@muttmuure
Copy link
Contributor

  • Are we sure that this isn't restricted to only the blank unread chats that appear when someone looks you up on New Expensify?
  • If it is that, I think we plan to remove that behavior. Therefore it might seem silly to fix a bug with something that might not be there.
  • If this is something more broad, as in all DMs in Workspaces, I agree this is a bug that we should fix.

@melvin-bot melvin-bot bot removed the Overdue label Jun 16, 2023
@iwiznia
Copy link
Contributor

iwiznia commented Jun 16, 2023

Are we sure that this isn't restricted to only the blank unread chats that appear when someone looks you up on New Expensify?

I don't get this... can you rephrase? What is a blank unread chat?

If this is something more broad, as in all DMs in Workspaces, I agree this is a bug that we should fix.

Very confused by this... the tooltip is not showing for workspace chats, that's the bug.

@melvin-bot melvin-bot bot added the Overdue label Jun 19, 2023
@muttmuure
Copy link
Contributor

I don't get this... can you rephrase? What is a blank unread chat?

When someone searches your name on New Expensify and clicks your name in Search, a chat with them appears in your LHN

@melvin-bot melvin-bot bot removed the Overdue label Jun 19, 2023
@muttmuure
Copy link
Contributor

My question is:

  • Does this affect those kinds of chats I have just described above or does it affect all workspace chats?

It sounds like you are confirming that it affects all workspace chats

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot
Copy link

melvin-bot bot commented Jun 22, 2023

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

@hoangzinh
Copy link
Contributor

Proposal

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

Web - New tooltip is not displayed over workspace direct chats and general workspace chat name

What is the root cause of that problem?

There are 2 issue here

  1. New tooltip is not displayed over workspace direct chats
    Take a look at condition to render SubscriptAvatar component. If it's a WS chat, it will render SubscriptAvatar component in the LHN. And in SubscriptAvatar component, currently we only render normal Tooltip

    <Tooltip text={props.mainTooltip}>
    <View>
    <Avatar
    source={props.mainAvatar.source}
    size={props.size === CONST.AVATAR_SIZE.SMALL ? CONST.AVATAR_SIZE.SMALL : CONST.AVATAR_SIZE.DEFAULT}
    name={props.mainAvatar.name}
    type={props.mainAvatar.type}
    />
    </View>
    </Tooltip>
    => We won't see new Tooltip if it's a WS chat.

  2. New tooltip is not displayed over general workspace chat name
    In LHN, we're using component DisplayNames to render WS chat name (and other reports too). And if it's a WS chat. It will render fullTitle according to condition here

    shouldUseFullTitle={
    optionItem.isChatRoom || optionItem.isPolicyExpenseChat || optionItem.isTaskReport || optionItem.isThread || optionItem.isMoneyRequestReport
    }

    Then we only render it as Text only in DisplayNames
    {this.props.shouldUseFullTitle
    ? this.props.fullTitle

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

  1. New tooltip is not displayed over workspace direct chats
    To solve this problem, we need to replace old Tooltip by new UserDetailsTooltip with the props accountID={props.mainAvatar.id} here

  2. New tooltip is not displayed over general workspace chat name
    Same as above, we need to wrap the text only by UserDetailsTooltip in this line

@thesahindia
Copy link
Member

@hoangzinh's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

@melvin-bot
Copy link

melvin-bot bot commented Jun 24, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Jun 26, 2023
@muttmuure
Copy link
Contributor

@stitesExpensify ready to assign @hoangzinh

@melvin-bot melvin-bot bot removed the Overdue label Jun 27, 2023
@0xmiros
Copy link
Contributor

0xmiros commented Jun 27, 2023

This might be fixed in #20512 which updates avatar patterns and tooltip for all types of chats.

@Christinadobrzyn
Copy link
Contributor

hey @thesahindia! Could you check and see if this issue will be fixed with this job? #21683

@hoangzinh
Copy link
Contributor

This might be fixed in #20512 which updates avatar patterns and tooltip for all types of chats.

@0xmiroslav This GH issue has 2 issues. Currently, I think the PR that you mentioned above only fix the 1st issue

@0xmiros
Copy link
Contributor

0xmiros commented Jun 27, 2023

At least put on hold until that PR is merged and deployed

@melvin-bot
Copy link

melvin-bot bot commented Jun 27, 2023

@stitesExpensify @muttmuure @thesahindia 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!

@stitesExpensify stitesExpensify changed the title [$1000] Web - New tooltip is not displayed over workspace direct chats and general workspace chat name [HOLD https://github.com/Expensify/App/pull/20512 $1000] Web - New tooltip is not displayed over workspace direct chats and general workspace chat name Jun 28, 2023
@stitesExpensify
Copy link
Contributor

Added a hold for #20512. When that is merged we can come back and see if this is still happening!

@melvin-bot melvin-bot bot added the Overdue label Jun 30, 2023
@muttmuure
Copy link
Contributor

Not overdue. Held

@melvin-bot melvin-bot bot removed the Overdue label Jul 3, 2023
@muttmuure muttmuure added Weekly KSv2 and removed Daily KSv2 labels Jul 3, 2023
@melvin-bot melvin-bot bot added the Overdue label Jul 11, 2023
@stitesExpensify
Copy link
Contributor

hold

@melvin-bot melvin-bot bot removed the Overdue label Jul 11, 2023
@melvin-bot melvin-bot bot added the Overdue label Jul 20, 2023
@stitesExpensify
Copy link
Contributor

This is the intended functionality right?
2023-07-21_12-52-38

@melvin-bot melvin-bot bot removed the Overdue label Jul 21, 2023
@0xmiros
Copy link
Contributor

0xmiros commented Jul 21, 2023

This is the intended functionality right? 2023-07-21_12-52-38

Right

@stitesExpensify
Copy link
Contributor

Cool, then this is fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
None yet
Development

No branches or pull requests