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 2022-12-20] While resizing, a white window background is shown #13085

Closed
Julesssss opened this issue Nov 28, 2022 · 24 comments
Closed
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 Reviewing Has a PR in review

Comments

@Julesssss
Copy link
Contributor

Julesssss commented Nov 28, 2022

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:

  • Resize the web browser window while in dark mode

Expected Result:

  • No obvious white background colour should be displayed

Actual Result:

  • An obvious white background colour is displayed
Screen.Recording.2022-11-24.at.11.42.55.mov

Platform:

Where is this issue occurring?

  • Web ✅

Version Number:
Reproducible in staging?: True
Reproducible in production?: True soon
Email or phone of affected tester (no customers): all

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019db029d99d78265b
  • Upwork Job ID: 1606391966285721600
  • Last Price Increase: 2022-12-23
@Julesssss Julesssss added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 28, 2022

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

@s77rt
Copy link
Contributor

s77rt commented Nov 28, 2022

I think @shawnborton suggestion will fix this bug.
However if the theme can be dynamically changed an event listener will be required to update the body CSS

@adelekennedy
Copy link

@Julesssss this looks like a dupe of this issue here from this comment it sounds like @grgia is already working on this - should we close this issue?

@melvin-bot melvin-bot bot removed the Overdue label Dec 2, 2022
@Julesssss
Copy link
Contributor Author

Good spot, but based I think they might be separate issues. Let's keep them separate until the solutions come in

@grgia
Copy link
Contributor

grgia commented Dec 2, 2022

Are you working on this one @Julesssss ? I have a somewhat hacky fix that I'm looking for a better solution for this PR

@Julesssss
Copy link
Contributor Author

Hey @grgia, nah I kept myself assigned in case this was made external, but feel free to take it over if you're working on a fix.

@melvin-bot melvin-bot bot removed the Overdue label Dec 5, 2022
@grgia grgia self-assigned this Dec 5, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 8, 2022

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

@melvin-bot melvin-bot bot added the Overdue label Dec 8, 2022
@grgia
Copy link
Contributor

grgia commented Dec 8, 2022

Whoops, in review!

@melvin-bot melvin-bot bot removed the Overdue label Dec 8, 2022
@grgia grgia added the Reviewing Has a PR in review label Dec 8, 2022
@Santhosh-Sellavel
Copy link
Collaborator

Please assign me this issue to track payment for this PR Review Thanks!

@adelekennedy adelekennedy added the Internal Requires API changes or must be handled by Expensify staff label Dec 8, 2022
@adelekennedy
Copy link

assigned @Santhosh-Sellavel for PR review payment!

@francoisl
Copy link
Contributor

francoisl commented Dec 9, 2022

The QA team sent this video, seems like we still show a white background on Windows. Any ideas or thoughts if we should make this a blocker?

13278.Web.mp4

@Santhosh-Sellavel
Copy link
Collaborator

@francoisl
On Mac, I don't get a white screen when I try to resize the browser or dragging, but I get to try to adjust the screen size in inspect mode. I am not sure we might be able to fix this.

On Staging

Screen.Recording.2022-12-10.at.12.14.50.AM.mov

On Local, unable to reproduce at all

Screen.Recording.2022-12-10.at.12.22.15.AM.mov

cc: @grgia

@francoisl
Copy link
Contributor

Yeah same here, which makes me think it's somehow specific to Windows. Anyway, we can look into this later, let's not block the deploy for this.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Dec 13, 2022
@melvin-bot melvin-bot bot changed the title While resizing, a white window background is shown [HOLD for payment 2022-12-20] While resizing, a white window background is shown Dec 13, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 13, 2022

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:

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
  • 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

@melvin-bot
Copy link

melvin-bot bot commented Dec 13, 2022

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:

  • [@Santhosh-Sellavel / @Julesssss / @grgia] The PR that introduced the bug has been identified. Link to the PR:
  • [@Santhosh-Sellavel / @Julesssss / @grgia] 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:
  • [@Santhosh-Sellavel / @Julesssss / @grgia] 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:
  • [] 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:

@Julesssss
Copy link
Contributor Author

No need for regression test, this was not a regression but an improvement.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 20, 2022
@Santhosh-Sellavel
Copy link
Collaborator

@grgia or @Julesssss can you add a bz here

@grgia
Copy link
Contributor

grgia commented Dec 22, 2022

Dragging you back @adelekennedy !!

@Santhosh-Sellavel
Copy link
Collaborator

@grgia seems @adelekennedy is on OOO can you assign someone else, thanks!

@mallenexpensify mallenexpensify added Internal Requires API changes or must be handled by Expensify staff and removed Internal Requires API changes or must be handled by Expensify staff labels Dec 23, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 23, 2022

Job added to Upwork: https://www.upwork.com/jobs/~019db029d99d78265b

@melvin-bot
Copy link

melvin-bot bot commented Dec 23, 2022

Current assignee @Santhosh-Sellavel is eligible for the Internal assigner, not assigning anyone new.

@mallenexpensify
Copy link
Contributor

@Santhosh-Sellavel
Copy link
Collaborator

@mallenexpensify Done!

@mallenexpensify
Copy link
Contributor

Paid @Santhosh-Sellavel $1000. Closing cuz I don't think we need regression test steps adds per @Julesssss 👎 above

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 Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

7 participants