Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.4k stars 2.79k forks source link

[HOLD Safari 17.4][$1000] [mWeb/Safari] Fix safari unfocused selection prop #26239

Closed thienlnam closed 2 months ago

thienlnam commented 1 year ago

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:

Break down in numbered steps

  1. We have a bug that is specific to mWeb/Safari that causes us to continue making safari specific updates in our code

https://github.com/Expensify/App/blob/bb2386beacf38d83ec438291bd4ca3684e9bf0ad/src/pages/home/report/ReportActionItemMessageEdit.js#L120-L122

https://github.com/Expensify/App/pull/24369/commits/f74dbb1afe2cbfe35fb167c8d87ccdfd18883449

  1. To reduce the amount of platform specific code in our repo - let's aim to get this fixed upstream. Creating this issue to track for when we can do this

Platforms:

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

Version Number: Reproducible in staging?: Reproducible in production?: 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 Expensify/Expensify Issue URL: Issue reported by: Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b2f48b6c5a3887bd
  • Upwork Job ID: 1700115117826949120
  • Last Price Increase: 2024-06-05
  • Automatic offers:
    • getusha | Contributor | 102518816
melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

thienlnam commented 1 year ago

Adding monthly as an upstream fix might take a while

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @Christinadobrzyn (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

melvin-bot[bot] commented 1 year ago

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

michaelhaxhiu commented 1 year ago

As mentioned by @thienlnam, this will get fixed upstream.

To reduce the amount of platform specific code in our repo - let's aim to get this fixed upstream. Creating this issue to track for when we can do this

We need a contributor to make the corresponding GH in that repository and then we'll collectively push to get this across the finish line. πŸ‘

ntdiary commented 1 year ago

Ah, what a coincidence! This is exactly the code I added previously. And actually, this is a Safari/Webkit bug, I submitted a PR to Webkit before, but had to put it on hold because I needed to fix other external issues first to make a living. I've been thinking I could fix it once I have some free time in the next couple months. :joy:

michaelhaxhiu commented 1 year ago

@ntdiary You want to submit the GH on the upstream repo to drive this through β›³ ?

ntdiary commented 1 year ago

Sure, this is my previous PR. I wanted to quickly fix it with a small change, but based on their feedback, I think it would be better to refactor it systematically, which will take more time.

melvin-bot[bot] commented 1 year ago

@ntdiary @michaelhaxhiu 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[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@ntdiary, @michaelhaxhiu Eep! 4 days overdue now. Issues have feelings too...

ntdiary commented 1 year ago

Hi, @michaelhaxhiu, does this issue need a daily label? Or could we change it to a weekly label instead?

michaelhaxhiu commented 1 year ago

@ntdiary good shout - we can switch it to Weekly or Monthly after the upstream PR is made & linked here. Did that step happen?

melvin-bot[bot] commented 1 year ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

ntdiary commented 1 year ago

This is a PR that was created previously, I plan to continue pushing it forward. : )

melvin-bot[bot] commented 1 year ago

@ntdiary @michaelhaxhiu this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

melvin-bot[bot] commented 1 year ago

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

michaelhaxhiu commented 1 year ago

Thanks @ntdiary :)

I think this is definitely worth finishing off. @thienlnam do you agree with the bounty price on this btw (e.g. it's fair for the effort and time involved all things considered)?

I'm assigning you since you made this GH and there's no engineer.

melvin-bot[bot] commented 1 year ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

thienlnam commented 1 year ago

Issues like this will take longer just due to the nature of having to get code accepted into a different repo that we don't really have control over. That being said, it would be nice to get this done as we have a few hacks added in our codebase to make a workaround for this.

Let's increase this to $1000 and just see where it goes

melvin-bot[bot] commented 1 year ago

Upwork job price has been updated to $1000

melvin-bot[bot] commented 1 year ago

@ntdiary @michaelhaxhiu @thienlnam this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

Thanks!

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

thienlnam commented 1 year ago

This is an upstream fix and will take time

michaelhaxhiu commented 1 year ago

I'm re-assigning this to another BZ as part of my preparation for Sabbatical (starting Friday).

Next steps:

melvin-bot[bot] commented 1 year ago

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

ntdiary commented 1 year ago

Note: I need to refactor that PR first, then request review. I may have time to do it this weekend. : )

melvin-bot[bot] commented 1 year ago

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

ntdiary commented 1 year ago

no overdue

melvin-bot[bot] commented 1 year ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

alexpensify commented 12 months ago

@ntdiary any update on the PR refactor?

ntdiary commented 12 months ago

@alexpensify, sorry for the delay, I got stuck reviewing other high priority issues the past few days. This is still under cross-browser testing (trying to align behaviors with Chrome and Firefox). Hope to be able to push it out over the weekend. πŸ’ͺ

alexpensify commented 12 months ago

Awesome, keep us posted if you need a hand or review.

melvin-bot[bot] commented 11 months ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

ntdiary commented 11 months ago

Not overdue. I pushed a revision with more targeted code (working well locally). If the community feels it's not good, I will try the other two solutions:

Now, let's wait for review feedback (maybe a few days). : )

alexpensify commented 11 months ago

Thank you for the update, keep us posted on how it's going.

alexpensify commented 11 months ago

@ntdiary - are you getting feedback? If no, do you need me to reach out to the repo maintainers? I can do so early next week.

ntdiary commented 11 months ago

@alexpensify, I haven't gotten any feedback yet. I just tagged them in the PR. If there's no feedback, I can ask again in the WebKit Slack channel this weekend. : )

melvin-bot[bot] commented 11 months ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

ntdiary commented 11 months ago

Not overdue, have got modification feedback in that PR, will push again later.

ntdiary commented 11 months ago

got some feedbacks again, will polish the PR later.

alexpensify commented 11 months ago

Daily Update: Looks like there is movement in the PR.

melvin-bot[bot] commented 11 months ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

alexpensify commented 11 months ago

Daily update: The PR has some requests but we are moving forward!

ntdiary commented 11 months ago

In discussion, plan to push again within 24 hours.

alexpensify commented 11 months ago

Let's go! Thanks for the update.

ntdiary commented 11 months ago

Just re-pushed a few minutes ago, because another high priority migration issue delayed this issue. πŸ˜