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 2023-03-16] Make the tooltip component accept any arbitrarily content #15126

Closed
hannojg opened this issue Feb 14, 2023 · 26 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 NewFeature Something to build that is a new item.

Comments

@hannojg
Copy link
Contributor

hannojg commented Feb 14, 2023

Part of #14180

Problem

Currently the <Tooltip /> component only accepts string to render. Its an issue when you want to render any other content than text in a tooltip.

Why is this important?

The emoji reactions feature wants to show the emoji and its senders in the tooltip.

Solution

Make the <Tooltip /> component accept a renderContent callback, as outlines here.

Note: No need to make this External or internal. Might not even need to be assigned as no payments will be issued. (External agency working on this)

@hannojg hannojg added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Feb 14, 2023
@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot closed this as completed Feb 14, 2023
@MelvinBot
Copy link

📣 @hannojg! 📣
Please report bugs or suggest features in the #expensify-open-source Slack channel, don't directly open issues in this repo!
Instructions here to join the channel 📖

@MelvinBot
Copy link

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

@mountiny mountiny reopened this Feb 14, 2023
@mountiny mountiny assigned hannojg and unassigned sonialiap Feb 14, 2023
@Expensify Expensify deleted a comment from Aaqib925 Feb 14, 2023
@marcaaron marcaaron added NewFeature Something to build that is a new item. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Feb 15, 2023
@MelvinBot
Copy link

@hannojg Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Feb 17, 2023
@melvin-bot melvin-bot bot added Reviewing Has a PR in review and removed Overdue labels Feb 21, 2023
@hannojg
Copy link
Contributor Author

hannojg commented Feb 21, 2023

PR is up for review: #15325

@MelvinBot
Copy link

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

@MelvinBot
Copy link

@hannojg Eep! 4 days overdue now. Issues have feelings too...

@hannojg
Copy link
Contributor Author

hannojg commented Mar 5, 2023

Currently fixing the PR as suggested by the review comments. Should be good for review on Monday morning 😊

@mountiny mountiny added the Bug Something is broken. Auto assigns a BugZero manager. label Mar 7, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@mountiny
Copy link
Contributor

mountiny commented Mar 7, 2023

Could we pay @s77rt $1000 for the PR testing and review please?

@melvin-bot melvin-bot bot added the Awaiting Payment Auto-added when associated PR is deployed to production label Mar 9, 2023
@melvin-bot melvin-bot bot changed the title Make the tooltip component accept any arbitrarily content [HOLD for payment 2023-03-16] Make the tooltip component accept any arbitrarily content Mar 9, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Mar 9, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

MelvinBot commented Mar 9, 2023

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

If no regressions arise, payment will be issued on 2023-03-16. 🎊

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@MelvinBot
Copy link

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:

  • [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
  • [@s77rt] 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:
  • [@s77rt] 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:
  • [@dylanexpensify] Determine if we should create a regression test for the bug.
  • [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to the appropriate location to ensure the same bug will not reach production again.
  • [@dylanexpensify] Review the proposed regression test steps and location.
  • [@dylanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@MelvinBot
Copy link

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

@s77rt
Copy link
Contributor

s77rt commented Mar 9, 2023

I don't think the checklist applies here since this is a feature request. Let me know if any action is required here.

@mountiny
Copy link
Contributor

mountiny commented Mar 9, 2023

Yep sounds good to me, no action required

@dylanexpensify dylanexpensify removed their assignment Mar 14, 2023
@dylanexpensify dylanexpensify added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Mar 14, 2023
@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Mar 14, 2023
@dylanexpensify
Copy link
Contributor

@tjferriss I'm heading OOO this week, mind tackling this one for me?? TY! 🙇

@Expensify Expensify deleted a comment from MelvinBot Mar 14, 2023
@melvin-bot melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Mar 15, 2023
@tjferriss
Copy link
Contributor

I've created the UpWorks job to process payment. I want to make sure I have this right:

  • External issue reporter @hannojg $250
  • Contributor that fixed the issue @hannojg $1000
  • Contributor+ that helped on the issue and/or PR @s77rt $1000

Does that look right to everyone? If so, can you please apply here: https://www.upwork.com/ab/applicants/1636137881404239872/job-details

Based on this comment, it sounds like all that remains is to get the payment processed. I tend to agree.

@s77rt
Copy link
Contributor

s77rt commented Mar 15, 2023

@tjferriss The upwork job is private

@tjferriss
Copy link
Contributor

Are you sure? It it telling me it's public right now: https://www.upwork.com/jobs/~013dc6542fe5fba104

Screenshot 2023-03-15 at 8 21 31 PM

@s77rt
Copy link
Contributor

s77rt commented Mar 16, 2023

@tjferriss That is a different link 😅 I have applied!

@roryabraham
Copy link
Contributor

@tjferriss
Copy link
Contributor

@s77rt I just accepted you in Upworks.

@hannojg sorry for the confusion, it looks like payment is handled through Margelo.

@melvin-bot melvin-bot bot added the Overdue label Mar 20, 2023
@tjferriss
Copy link
Contributor

Payment processed. Closing.

@melvin-bot melvin-bot bot removed the Overdue label Mar 20, 2023
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 NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

9 participants