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 for payment 2024-04-03][$250] Chat - Missing Tooltip for Video Expand Button #36890

Closed
1 of 6 tasks
lanitochka17 opened this issue Feb 20, 2024 · 34 comments
Closed
1 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff

Comments

@lanitochka17
Copy link

lanitochka17 commented Feb 20, 2024

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


Version Number: 1.4.43-2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers): abebemiherat@gmail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1, Navigate to the conversion section
2, Add a video. The expand button appears alongside the video

Expected Result:

The expand button for videos should display a tooltip

Actual Result:

The expand button for videos does not display a tooltip

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6385596_1708427639159.Screen_Recording_2024-02-19_at_10.59.45_PM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e702a28b240448c2
  • Upwork Job ID: 1759949924144463872
  • Last Price Increase: 2024-02-20
  • Automatic offers:
    • ntdiary | Reviewer | 0
    • neonbhai | Contributor | 0
Issue OwnerCurrent Issue Owner: @muttmuure
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment External Added to denote the issue can be worked on by a contributor labels Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

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

@melvin-bot melvin-bot bot changed the title Chat - Missing Tooltip for Video Expand Button [$500] Chat - Missing Tooltip for Video Expand Button Feb 20, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Feb 20, 2024
@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Feb 20, 2024
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.

Copy link

melvin-bot bot commented Feb 20, 2024

Triggered auto assignment to @arosiclair (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@MahmudjonToraqulov
Copy link
Contributor

Proposal

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

Chat - Missing Tooltip for Video Expand Button

What is the root cause of that problem?

We are missing the logic for showing a tooltip.

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

We should wrap the icon here with Tooltip:

 <Tooltip text={translate('videoPlayer.expand')}>
 ...
 </Tooltip>

What alternative solutions did you explore? (Optional)

N/A

@neonbhai
Copy link
Contributor

neonbhai commented Feb 20, 2024

Proposal

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

Chat - Missing Tooltip for Video Expand Button

What is the root cause of that problem?

We have the Tooltip component outside the Hoverable Component here

This is the reason none of the tooltips are showing.

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

We should move the Tooltip component inside the Hoverable component.

        <Hoverable>
            {(isHovered) => (
                <Tooltip
                    text={tooltipText}
                    shouldForceRenderingBelow={shouldForceRenderingTooltipBelow}
                >
                   ...
                </Tooltip>
              )}
        </Hoverable>

Result

This works perfectly:

Screen.Recording.2024-02-20.at.8.33.27.PM.mov

@arosiclair arosiclair removed 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 labels Feb 20, 2024
@arosiclair
Copy link
Contributor

Looks like this IconButton is only used in the video player. Which is a bit confusing since there is another IconButton we use elsewhere, but that is a separate problem.

Since it's only for the video player I'm removing the blocker label. I'm also lowering the bounty to $250 since it is so simple.

@arosiclair arosiclair added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. and removed DeployBlockerCash This issue or pull request should block deployment Engineering Hourly KSv2 labels Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

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

@arosiclair arosiclair added the External Added to denote the issue can be worked on by a contributor label Feb 20, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 20, 2024
Copy link

melvin-bot bot commented Feb 20, 2024

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

@melvin-bot melvin-bot bot added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor labels Mar 19, 2024
Copy link

melvin-bot bot commented Mar 19, 2024

Current assignee @ntdiary is eligible for the Internal assigner, not assigning anyone new.

@muttmuure
Copy link
Contributor

Seems like LOW #vip-vsb

@melvin-bot melvin-bot bot added the Overdue label Mar 25, 2024
@arosiclair
Copy link
Contributor

PR is on staging

@melvin-bot melvin-bot bot removed the Overdue label Mar 25, 2024
@arosiclair
Copy link
Contributor

PR hit prod on 2024-03-27

@arosiclair arosiclair added the Awaiting Payment Auto-added when associated PR is deployed to production label Mar 29, 2024
@arosiclair arosiclair changed the title [$250] Chat - Missing Tooltip for Video Expand Button [HOLD for payment 2024-04-03][$250] Chat - Missing Tooltip for Video Expand Button Mar 29, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 1, 2024
@arosiclair
Copy link
Contributor

Couple more days until payment

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Apr 1, 2024
@arosiclair
Copy link
Contributor

Ready for payment @muttmuure

@melvin-bot melvin-bot bot removed the Overdue label Apr 8, 2024
@muttmuure
Copy link
Contributor

@ntdiary - $250 C+
@neonbhai - $250 C

@muttmuure
Copy link
Contributor

@neonbhai paid, just waiting for @ntdiary to accept offer

@ntdiary
Copy link
Contributor

ntdiary commented Apr 11, 2024

@neonbhai paid, just waiting for @ntdiary to accept offer

@muttmuure, Thank you! Accepted it. 😄

@melvin-bot melvin-bot bot added the Overdue label Apr 12, 2024
@arosiclair
Copy link
Contributor

Sounds like this is done ^

@melvin-bot melvin-bot bot removed the Overdue label Apr 12, 2024
@ntdiary
Copy link
Contributor

ntdiary commented Sep 6, 2024

Hi, @muttmuure, I just noticed that this issue hasn't been paid yet.
Since I've switched to NewDot for payments and no longer use upwork. Can I request payment through NewDot? 😂

@ntdiary
Copy link
Contributor

ntdiary commented Sep 10, 2024

Hi, @muttmuure, I just noticed that this issue hasn't been paid yet. Since I've switched to NewDot for payments and no longer use upwork. Can I request payment through NewDot? 😂

Hi, @arosiclair, can you please reopen this issue so we can process the payment? 😄
image

cc @muttmuure

@mallenexpensify
Copy link
Contributor

@ntdiary can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~021836106341219430298

Your assigned date on this issue was a handful of days before your eligibility date
image

image

@ntdiary
Copy link
Contributor

ntdiary commented Sep 18, 2024

@mallenexpensify, Thank you! I have accepted the new job, and ended the previous contract. 😄

@muttmuure
Copy link
Contributor

Paid - sorry for the oversight!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff
Projects
No open projects
Status: CRITICAL
Development

No branches or pull requests

7 participants