You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2023-09-04] [$1000] BUG: Rectangular white container surrounds the Google sign-in button #25638
changed the title [-]BUG: Rectangular white container surrounds the Google sign-in button[/-][+][$1000] BUG: Rectangular white container surrounds the Google sign-in button[/+]on Aug 22, 2023
Please re-state the problem that we are trying to solve in this issue.
The "Sign in with Google" button, when embedded in the Expensify web application, displayed an unwanted white background. This visual discrepancy was inconsistent with the desired dark theme of the application.
What is the root cause of that problem?
The root (:root) of the Expensify web application's document has the color-scheme: dark property set. This indicates to the browser that the content is optimized for dark color themes. The Google Sign-in button, being a third-party component rendered within an iframe, adapts its appearance based on the surrounding context's color scheme. Due to the color-scheme: dark property, the dynamically generated iframe for the Google Sign-in button defaulted to a white background, leading to the observed issue.
By setting the color-scheme to light for the specific container of the Google Sign-in button, it is influenced to adopt a light theme, ensuring its appearance is consistent with the expected design and eliminating the unwanted white background.
Result:
What alternative solutions did you explore? (Optional)
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.57-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 2023-09-04. 🎊
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
For reference, here are some details about the assignees on this issue:
@parasharrajatdoes not require payment (Eligible for Manual Requests)
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:
[@parasharrajat] The PR that introduced the bug has been identified. Link to the PR:
[@parasharrajat] 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:
[@parasharrajat] 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:
[@parasharrajat] Determine if we should create a regression test for this bug.
[@parasharrajat] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
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:
[@parasharrajat] 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, it seems this issue was known during the PR review.
[@parasharrajat] Determine if we should create a regression test for this bug. Yes
[@parasharrajat] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
Regression Test Steps
Go to the sign-in page.
Notice the google sign-in button on the dark theme.
Verify that no rectangular white container surrounds the Google sign-in button.
Activity
melvin-bot commentedon Aug 22, 2023
Triggered auto assignment to @greg-schroeder (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.melvin-bot commentedon Aug 22, 2023
Bug0 Triage Checklist (Main S/O)
Platformsin OP are ✅)[-]BUG: Rectangular white container surrounds the Google sign-in button[/-][+][$1000] BUG: Rectangular white container surrounds the Google sign-in button[/+]melvin-bot commentedon Aug 22, 2023
Job added to Upwork: https://www.upwork.com/jobs/~013a33ff67a596f404
melvin-bot commentedon Aug 22, 2023
Current assignee @greg-schroeder is eligible for the External assigner, not assigning anyone new.
melvin-bot commentedon Aug 22, 2023
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (
External)rayane-d commentedon Aug 22, 2023
Proposal
Please re-state the problem that we are trying to solve in this issue.
The "Sign in with Google" button, when embedded in the Expensify web application, displayed an unwanted white background. This visual discrepancy was inconsistent with the desired dark theme of the application.
What is the root cause of that problem?
The root
(:root)of the Expensify web application's document has thecolor-scheme: darkproperty set. This indicates to the browser that the content is optimized for dark color themes. The Google Sign-in button, being a third-party component rendered within aniframe, adapts its appearance based on the surrounding context's color scheme. Due to thecolor-scheme: darkproperty, the dynamically generatediframefor the Google Sign-in button defaulted to a white background, leading to the observed issue.App/web/index.html
Lines 34 to 36 in e3074c4
What changes do you think we should make in order to solve the problem?
To address this visual discrepancy, the following code change need to be implemented:
In the styles for the container of the Google Sign-in button,
App/src/styles/styles.js
Lines 3638 to 3648 in e3074c4
we need to introduce
color-schemeproperty and set it tolight.By setting the
color-schemetolightfor the specific container of the Google Sign-in button, it is influenced to adopt a light theme, ensuring its appearance is consistent with the expected design and eliminating the unwanted white background.Result:
What alternative solutions did you explore? (Optional)
N/A
melvin-bot commentedon Aug 22, 2023
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.
If a regression has occurred and you are the assigned CM follow the instructions here.
If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.
15 remaining items
melvin-bot commentedon Aug 28, 2023
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon Aug 28, 2023
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.57-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 2023-09-04. 🎊
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.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
melvin-bot commentedon Aug 28, 2023
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:
greg-schroeder commentedon Sep 5, 2023
Yesterday was a holiday, will take care of this today!
parasharrajat commentedon Sep 5, 2023
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:
[@parasharrajat] The PR that introduced the bug has been identified. Link to the PR: Apple & Google Sign In #23673
[@parasharrajat] 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: Apple & Google Sign In #23673 (comment)
[@parasharrajat] 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, it seems this issue was known during the PR review.
[@parasharrajat] Determine if we should create a regression test for this bug. Yes
[@parasharrajat] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
Regression Test Steps
Do you agree 👍 or 👎 ?
greg-schroeder commentedon Sep 6, 2023
Issue Participants:
Issue reported by: @rayane-djouah
Contributor: @rayane-djouah
C+: @parasharrajat
Was this issue merged in time to be eligible for the speed bonus? ✅
Were their any regressions? ❌
Payment summary:
Reporter: $250
Contributor: $1,500
C+: 1,500
greg-schroeder commentedon Sep 6, 2023
Filing regression test. You're in the clear to make a manual request for payment @parasharrajat. Closing.
parasharrajat commentedon Sep 11, 2023
Payment requested as per #25638 (comment)
JmillsExpensify commentedon Sep 21, 2023
$1,500 payment for @parasharrajat approved based on BZ summary.