[HOLD for payment 2022-12-20] [$500] BUG: Background color does not cover status bar area

[kavimuru]

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:

1. Open NewDot and login
2. Go to ay chat
3. Long press on any comment to open the docked menu

Expected Result:

Background color should cover status bar on top

Actual Result:

Does not cover the area

Workaround:

Visual

Platform:

Where is this issue occurring?

• Mobile Web - Safari

Version Number: 1.2.19-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Expensify/Expensify Issue URL:
Issue reported by: @roryabraham
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666776603827639

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01003b0856ebbae387
• Upwork Job ID: 1597375398625361920

[melvin-bot]

Triggered auto assignment to @sakluger (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot]

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

[sakluger]

I was able to reproduce. Waiting on the Slack discussion to decide whether to label external or no.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[tgolen]

I'm not really sure that this is a bug or something we want to fix. When that menu is open, there is an opacity overlay over the rest of the app, so it's not really a "background color" that we would want to extend to the status bar area. I'm going to close this, but also tagging @Expensify/design just for confirmation on my theory.

[shawnborton]

Yeah, I don't think we can do much about that. Ideally the opacity/BG overlay would stretch over the status bar area but I don't think it's possible.

[roryabraham]

Ideally the opacity/BG overlay would stretch over the status bar area but I don't think it's possible.

I agree that this would be ideal, and I think it probably is possible? Reopening this

[roryabraham]

Or, maybe it's only possible on native but not mobile web. Let's see what proposals we get?

[melvin-bot]

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

[eVoloshchak]

Not overdue, waiting for proposals

[puneetlath]

@tgolen To help us clear out the large backlog of /App bugs, we're putting the spotlight every bug in the repo already than 4 weeks old. To help unblock the roadmap and get our bug pipeline back in equilibrium, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is
• For any that can't, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #bug-zero

[tgolen]

While this issue is more than 4 weeks old, it's only been open for proposals for a week. I'm going to wait another week to see if there are any proposals, but I think I will probably just end up closing this again.

[JmillsExpensify]

Nice! @sakluger you're currently assigned. Do you want to take this one, or should I assign myself to the issue?

[melvin-bot]

📣 @dnlfrst You have been assigned to this job by @JmillsExpensify!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[JmillsExpensify]

Let's get a PR up for this one. 50% bonus is merged within 3 days!

[sakluger]

@JmillsExpensify thanks for keeping this one moving forward, I can handle it from here!

[dnlfrst]

Let's get a PR up for this one. 50% bonus is merged within 3 days!

A first draft of the PR is now underway at #13215.

Please make sure to add a comment explaining that this line is for changing the status bar color above document.querySelector('meta[name=theme-color]').content (and link this issue), since the code is not self-explanatory

I suppose we still need to call props.onModalHide() and props.onModalShow() in the corresponding BaseModal methods

The draft already addresses both issues.

[dnlfrst]

🟢 on the proposal. I think we should also bump this issue up to $500 @JmillsExpensify.

There are a few things I see that I will address in the PR itself.

I received an offer on Upwork at $250, should I already accept that or does this bump need to apply before?

[tgolen]

You can probably just accept that job now and we can settle up later.

[sakluger]

Thanks for calling out the increased amount @dnlfrst, I had missed that comment. I've updated the price in this Github issue title. I can't change the offer amount in Upwork at this point, but we will make sure to pay out the correct amount via Upwork once the PR is merged and settled.

[sakluger]

Looks like the linked PR was merged but has not been deployed yet. I'm going to switch this one to a weekly while we wait to make sure there are no issue after deploy.

[melvin-bot]

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

• Adapt status bar color to modal backdrop #13215

If no regressions arise, payment will be issued on 2022-12-20. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

• External issue reporter reported by Rory, no external reporter
• Contributor that fixed the issue - @dnlfrst
• Contributor+ that helped on the issue and/or PR - @eVoloshchak

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

[melvin-bot]

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:

• [@eVoloshchak / @tgolen] The PR that introduced the bug has been identified. Link to the PR: NA
• [@eVoloshchak / @tgolen] 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: NA
• [@eVoloshchak / @tgolen] 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: NA
• [@sakluger] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

[sakluger]

This was merged 7 days after assignment, there is no bonus or penalty. I will pay out tomorrow as long as there are no regressions.

I started a conversation in Slack in #bug-zero around whether we need regression tests for general UI elements, I'll update here when I have more details from that thread.

[sakluger]

No new regression test steps needed for this since Applause looks for cosmetic problems when running through the full set of tests.

[sakluger]

Paid @dnlfrst and @eVoloshchak $500 each. 🎉

@eVoloshchak @tgolen could you two please help complete the checkboxes in this comment before we close the issue? Thanks!

[tgolen]

OK, sure. I've checked them off. I actually don't think they apply because this wasn't really a "bug" as much as it was a "new feature". It was never something that existed before, so there was nothing that broke. It was just not something that was ever done in the original design of the app.

[sakluger]

Ah, that makes sense! I wasn't sure if it was something we broke or hadn't considered before, didn't want to make any assumptions. Thanks for the checkmarks 👌