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.36k stars 2.78k forks source link

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

Closed kavimuru closed 1 year ago

kavimuru 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:

  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?

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

sakluger commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

tgolen commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

eVoloshchak commented 1 year ago

Not overdue, waiting for proposals

puneetlath commented 1 year ago

@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:

tgolen commented 1 year ago

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.

dnlfrst commented 1 year ago

Proposal

In Safari, it is possible to modify the status bar's color using the theme-color value for the name attribute of the <meta> tag. In particular, I propose a patch along the lines of:

diff --git a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js
index 49e1a2f97..b1a92168c 100644
--- a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js
+++ b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js
@@ -1,4 +1,5 @@
 import React from 'react';
+import { Helmet } from "react-helmet";
 import {
     Dimensions,
 } from 'react-native';
@@ -288,6 +289,11 @@ class PopoverReportActionContextMenu extends React.Component {
     render() {
         return (
             <>
+                {this.state.isPopoverVisible ? (
+                    <Helmet>
+                        <meta name="theme-color" content="#E6E3E3" />
+                    </Helmet>
+                ) : null}
                 <PopoverWithMeasuredContent
                     isVisible={this.state.isPopoverVisible}
                     onClose={this.hideContextMenu}

This change gives the following results:

Result

https://user-images.githubusercontent.com/13868083/202913335-4c359c7f-b2a2-4086-af9f-d816630f56df.MP4

Please note that the final implementation would dynamically compute the required theme-color to account for different themes. One caveat is that the color change of the status bar is not animated, the backdrop of the popover is, though. This causes a slight visual misalignment that is also visible in the above video. Finally, the solution currently relies on react-helmet to dynamically inject the <meta> tag.

tgolen commented 1 year ago

I don't think that animation is causing too much of an issue. Rather than using react-helmet, can you please change your proposal to not use it?

dnlfrst commented 1 year ago

@tgolen I will make sure to accomplish the same as proposed without using react-helmet and get back to you as soon as possible.

dnlfrst commented 1 year ago

Proposal (Update)

The updated proposal now fixes the status bar color as follows:

From ef6a201350ace58dd1d9d5f9530cbefadff45d12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Fu=CC=88rst?= <dpfuerst@gmail.com>
Date: Tue, 22 Nov 2022 01:25:14 +0100
Subject: [PATCH] Adapt status bar color to modal backdrop

---
 .../PopoverReportActionContextMenu.js         |  6 ++++
 src/styles/StyleUtils.js                      | 35 +++++++++++++++++++
 web/index.html                                |  1 +
 3 files changed, 42 insertions(+)

diff --git a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js
index 2ac4cf1042..a9ef374a9d 100644
--- a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js
+++ b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js
@@ -9,6 +9,7 @@ import withLocalize, {withLocalizePropTypes} from '../../../../components/withLo
 import PopoverWithMeasuredContent from '../../../../components/PopoverWithMeasuredContent';
 import BaseReportActionContextMenu from './BaseReportActionContextMenu';
 import ConfirmModal from '../../../../components/ConfirmModal';
+import * as StyleUtils from '../../../../styles/StyleUtils';

 const propTypes = {
     /** Flag to check if the chat participant is Chronos */
@@ -148,6 +149,8 @@ class PopoverReportActionContextMenu extends React.Component {
             this.onPopoverHide = onHide;
         };
         this.getContextMenuMeasuredLocation().then(({x, y}) => {
+            document.querySelector('meta[name=theme-color]').content = StyleUtils.getThemeColor();
+
             this.setState({
                 cursorRelativePosition: {
                     horizontal: nativeEvent.pageX - x,
@@ -213,6 +216,9 @@ class PopoverReportActionContextMenu extends React.Component {
         if (_.isFunction(onHideActionCallback)) {
             this.onPopoverHideActionCallback = onHideActionCallback;
         }
+
+        document.querySelector('meta[name=theme-color]').content = '';
+
         this.setState({
             reportID: '0',
             reportAction: {},
diff --git a/src/styles/StyleUtils.js b/src/styles/StyleUtils.js
index d05acdfa8f..075e8124da 100644
--- a/src/styles/StyleUtils.js
+++ b/src/styles/StyleUtils.js
@@ -444,6 +444,40 @@ function getPaymentMethodMenuWidth(isSmallScreenWidth) {
     return {width: !isSmallScreenWidth ? variables.sideBarWidth - (margin * 2) : undefined};
 }

+/**
+ * Approximates the theme color for a modal based on the app's background color,
+ * the modal's backdrop, and the backdrop's opacity.
+ *
+ * @returns {String} The theme color as an RGB value.
+ */
+function getThemeColor() {
+    const hexadecimalToRGB = hexadecimal => _.map(hexadecimal.replace(/^#?([a-f\d])([a-f\d])([a-f\d])$/i,
+        (m, r, g, b) => `#${r}${r}${g}${g}${b}${b}`)
+        .substring(1).match(/.{2}/g), x => parseInt(x, 16));
+
+    const [backgroundR, backgroundG, backgroundB] = hexadecimalToRGB(themeColors.appBG);
+    const [backdropR, backdropG, backdropB] = hexadecimalToRGB(themeColors.modalBackdrop);
+
+    // This value is hardcoded in `BaseModal.js`.
+    const backdropOpacity = 0.5;
+
+    const normalizedBackdropR = backdropR / 255;
+    const normalizedBackdropG = backdropG / 255;
+    const normalizedBackdropB = backdropB / 255;
+    const normalizedBackgroundR = backgroundR / 255;
+    const normalizedBackgroundG = backgroundG / 255;
+    const normalizedBackgroundB = backgroundB / 255;
+
+    const normalizedApproximatedR = ((1 - backdropOpacity) * normalizedBackdropR) + (backdropOpacity * normalizedBackgroundR);
+    const normalizedApproximatedG = ((1 - backdropOpacity) * normalizedBackdropG) + (backdropOpacity * normalizedBackgroundG);
+    const normalizedApproximatedB = ((1 - backdropOpacity) * normalizedBackdropB) + (backdropOpacity * normalizedBackgroundB);
+    const approximatedR = Math.floor(normalizedApproximatedR * 255);
+    const approximatedG = Math.floor(normalizedApproximatedG * 255);
+    const approximatedB = Math.floor(normalizedApproximatedB * 255);
+
+    return `rgb(${approximatedR}, ${approximatedG}, ${approximatedB})`;
+}
+
 /**
  * Parse styleParam and return Styles array
  * @param {Object|Object[]} styleParam
@@ -561,6 +595,7 @@ export {
     getMiniReportActionContextMenuWrapperStyle,
     getKeyboardShortcutsModalWidth,
     getPaymentMethodMenuWidth,
+    getThemeColor,
     parseStyleAsArray,
     combineStyles,
     getPaddingLeft,
diff --git a/web/index.html b/web/index.html
index ca117f328d..638a139d4a 100644
--- a/web/index.html
+++ b/web/index.html
@@ -4,6 +4,7 @@
     <meta charset="utf-8" />
     <title>New Expensify</title>
     <meta name="description" content="Corporate cards, reimbursements, receipt scanning, invoicing, and bill pay. One app, all free.">
+    <meta name="theme-color" content="">
     <meta property="twitter:card" content="summary">
     <meta property="twitter:site" content="@expensify">
     <meta property="og:type" content="website">
-- 
2.23.0

The only remaining minor issue is that the backdrop opacity of the modal is hardcoded since it is hardcoded in BaseModal.js as well.

tgolen commented 1 year ago

Ah, that looks cool. Thanks for working on that! I like the direction of that proposal. There are a few things I think we can clean up in the PR (move opacity to the style/variables.js file, move the document methods to a web-only platform file, see if getThemeColor could be simplified at all, etc.) but that's on the right track that I was expecting.

@eVoloshchak Do you want to take a look at this proposal?

eVoloshchak commented 1 year ago

Proposal looks good to me, i didn't know this was possible, nice!

A couple of things

  1. I'm not sure this is needed for Android, i couldn't find models where status bar color could be changed. But there might be, so I'm not sure if we need to apply this only for Safari
  2. Would it be possible to have a more generalized approach? I suppose there are other cases where this bug might appear, opening any other fullscreen modal
dnlfrst commented 1 year ago

see if getThemeColor could be simplified at all

Not that I am aware of it. theme-color does not support the alpha channel, hence the need to approximate the opacity with RGB.

dnlfrst commented 1 year ago

I'm not sure this is needed for Android, i couldn't find models where status bar color could be changed. But there might be, so I'm not sure if we need to apply this only for Safari

Judging from the official documentation of Google Chrome (and CIU), the status bar color should also work on Android as long as the native dark mode is not enabled.

Would it be possible to have a more generalized approach? I suppose there are other cases where this bug might appear, opening any other fullscreen modal

That is a good point! I might need some guidance there since I am not too familiar with the code base.

dnlfrst commented 1 year ago

Proposal (Update)

@tgolen @eVoloshchak the updated proposal now fixes the status bar color in a more general way:

From 5c2fda45dabe5e33f46b9ec7c32ae5afa34cc930 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Fu=CC=88rst?= <dpfuerst@gmail.com>
Date: Sun, 27 Nov 2022 14:05:38 +0100
Subject: [PATCH] Adapt status bar color to modal backdrop

---
 src/components/Modal/BaseModal.js | 10 ++++++++++
 src/styles/StyleUtils.js          | 33 +++++++++++++++++++++++++++++++
 src/styles/variables.js           |  1 +
 web/index.html                    |  1 +
 4 files changed, 45 insertions(+)

diff --git a/src/components/Modal/BaseModal.js b/src/components/Modal/BaseModal.js
index 91b1867a6f..0de83c7faf 100644
--- a/src/components/Modal/BaseModal.js
+++ b/src/components/Modal/BaseModal.js
@@ -9,6 +9,8 @@ import themeColors from '../../styles/themes/default';
 import {propTypes as modalPropTypes, defaultProps as modalDefaultProps} from './modalPropTypes';
 import * as Modal from '../../libs/actions/Modal';
 import getModalStyles from '../../styles/getModalStyles';
+import getPlatform from '../../libs/getPlatform';
+import CONST from '../../CONST';

 const propTypes = {
     ...modalPropTypes,
@@ -53,6 +55,10 @@ class BaseModal extends PureComponent {
         if (callHideCallback) {
             this.props.onModalHide();
         }
+
+        if (this.props.fullscreen && getPlatform() === CONST.PLATFORM.WEB) {
+            document.querySelector('meta[name=theme-color]').content = '';
+        }
     }

     render() {
@@ -94,6 +100,10 @@ class BaseModal extends PureComponent {
                         Modal.setModalVisibility(true);
                     }
                     this.props.onModalShow();
+
+                    if (this.props.fullscreen && getPlatform() === CONST.PLATFORM.WEB) {
+                        document.querySelector('meta[name=theme-color]').content = StyleUtils.getThemeColor();
+                    }
                 }}
                 propagateSwipe={this.props.propagateSwipe}
                 onModalHide={this.hideModal}
diff --git a/src/styles/StyleUtils.js b/src/styles/StyleUtils.js
index d05acdfa8f..0dd0405a80 100644
--- a/src/styles/StyleUtils.js
+++ b/src/styles/StyleUtils.js
@@ -444,6 +444,38 @@ function getPaymentMethodMenuWidth(isSmallScreenWidth) {
     return {width: !isSmallScreenWidth ? variables.sideBarWidth - (margin * 2) : undefined};
 }

+/**
+ * Approximates the theme color for a modal based on the app's background color,
+ * the modal's backdrop, and the backdrop's opacity.
+ *
+ * @returns {String} The theme color as an RGB value.
+ */
+function getThemeColor() {
+    const backdropOpacity = variables.modalFullscreenBackdropOpacity;
+    const hexadecimalToRGB = hexadecimal => _.map(hexadecimal.replace(/^#?([a-f\d])([a-f\d])([a-f\d])$/i,
+        (m, r, g, b) => `#${r}${r}${g}${g}${b}${b}`)
+        .substring(1).match(/.{2}/g), x => parseInt(x, 16));
+
+    const [backgroundR, backgroundG, backgroundB] = hexadecimalToRGB(themeColors.appBG);
+    const [backdropR, backdropG, backdropB] = hexadecimalToRGB(themeColors.modalBackdrop);
+
+    const normalizedBackdropR = backdropR / 255;
+    const normalizedBackdropG = backdropG / 255;
+    const normalizedBackdropB = backdropB / 255;
+    const normalizedBackgroundR = backgroundR / 255;
+    const normalizedBackgroundG = backgroundG / 255;
+    const normalizedBackgroundB = backgroundB / 255;
+
+    const normalizedApproximatedR = ((1 - backdropOpacity) * normalizedBackdropR) + (backdropOpacity * normalizedBackgroundR);
+    const normalizedApproximatedG = ((1 - backdropOpacity) * normalizedBackdropG) + (backdropOpacity * normalizedBackgroundG);
+    const normalizedApproximatedB = ((1 - backdropOpacity) * normalizedBackdropB) + (backdropOpacity * normalizedBackgroundB);
+    const approximatedR = Math.floor(normalizedApproximatedR * 255);
+    const approximatedG = Math.floor(normalizedApproximatedG * 255);
+    const approximatedB = Math.floor(normalizedApproximatedB * 255);
+
+    return `rgb(${approximatedR}, ${approximatedG}, ${approximatedB})`;
+}
+
 /**
  * Parse styleParam and return Styles array
  * @param {Object|Object[]} styleParam
@@ -561,6 +593,7 @@ export {
     getMiniReportActionContextMenuWrapperStyle,
     getKeyboardShortcutsModalWidth,
     getPaymentMethodMenuWidth,
+    getThemeColor,
     parseStyleAsArray,
     combineStyles,
     getPaddingLeft,
diff --git a/src/styles/variables.js b/src/styles/variables.js
index 409c9be576..461995d4a0 100644
--- a/src/styles/variables.js
+++ b/src/styles/variables.js
@@ -42,6 +42,7 @@ export default {
     emojiLineHeight: 28,
     iouAmountTextSize: 40,
     mobileResponsiveWidthBreakpoint: 800,
+    modalFullscreenBackdropOpacity: 0.5,
     tabletResponsiveWidthBreakpoint: 1024,
     safeInsertPercentage: 0.7,
     sideBarWidth: 375,
diff --git a/web/index.html b/web/index.html
index ca117f328d..638a139d4a 100644
--- a/web/index.html
+++ b/web/index.html
@@ -4,6 +4,7 @@
     <meta charset="utf-8" />
     <title>New Expensify</title>
     <meta name="description" content="Corporate cards, reimbursements, receipt scanning, invoicing, and bill pay. One app, all free.">
+    <meta name="theme-color" content="">
     <meta property="twitter:card" content="summary">
     <meta property="twitter:site" content="@expensify">
     <meta property="og:type" content="website">
-- 
2.23.0
eVoloshchak commented 1 year ago

@dnlfrst, the more general approach is awesome!

Judging from the official documentation of Google Chrome (and CIU), the status bar color should also work on Android as long as the native dark mode is not enabled.

You are right, just confirmed it working on Chrome, my mistake was using the dark mode

I'm happy with the proposal functionality-wise, there's only a couple of stylistic issues that need to be addressed, @tgolen listed them all here:

Could you update your proposal with these? If you need help or more context - just tag me, I'll try to be more readily available

sakluger commented 1 year ago

Looks like this one is moving forward! I'm going to remove and re-add the external label to create an Upwork job.

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~01003b0856ebbae387

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

dnlfrst commented 1 year ago

Proposal (Update)

move opacity to the style/variables.js file

This is already part of the above proposal.

see if getThemeColor could be simplified at all

This is addressed with the below proposal.

move the document methods to a web-only platform file

Intuitively, I do not think that it makes sense to move the document.* calls to CustomStatusBar. Rather, I have now moved the web-specific calls to Modal. @eVoloshchak let me know what you think about this.

From 658586a0ccba9750bc0e8cd75e01bc00f633e737 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Fu=CC=88rst?= <dpfuerst@gmail.com>
Date: Tue, 29 Nov 2022 11:48:16 +0100
Subject: [PATCH] Adapt status bar color to modal backdrop

---
 src/components/Modal/index.web.js | 29 ++++++++++
 src/styles/StyleUtils.js          | 94 +++++++++++++++++++++++++++++--
 src/styles/variables.js           |  1 +
 web/index.html                    |  1 +
 4 files changed, 120 insertions(+), 5 deletions(-)
 create mode 100644 src/components/Modal/index.web.js

diff --git a/src/components/Modal/index.web.js b/src/components/Modal/index.web.js
new file mode 100644
index 0000000000..86a1610095
--- /dev/null
+++ b/src/components/Modal/index.web.js
@@ -0,0 +1,29 @@
+import React from 'react';
+import withWindowDimensions from '../withWindowDimensions';
+import BaseModal from './BaseModal';
+import {propTypes, defaultProps} from './modalPropTypes';
+import * as StyleUtils from '../../styles/StyleUtils';
+
+const Modal = props => (
+    <BaseModal
+            // eslint-disable-next-line react/jsx-props-no-spreading
+        {...props}
+        onModalHide={() => {
+            if (!props.fullscreen) { return; }
+
+            document.querySelector('meta[name=theme-color]').content = '';
+        }}
+        onModalShow={() => {
+            if (!props.fullscreen) { return; }
+
+            document.querySelector('meta[name=theme-color]').content = StyleUtils.getThemeColor();
+        }}
+    >
+        {props.children}
+    </BaseModal>
+);
+
+Modal.propTypes = propTypes;
+Modal.defaultProps = defaultProps;
+Modal.displayName = 'Modal';
+export default withWindowDimensions(Modal);
diff --git a/src/styles/StyleUtils.js b/src/styles/StyleUtils.js
index d05acdfa8f..fd7c89b813 100644
--- a/src/styles/StyleUtils.js
+++ b/src/styles/StyleUtils.js
@@ -202,6 +202,22 @@ function getBackgroundColorStyle(backgroundColor) {
     };
 }

+/**
+ * Converts a color in hexadecimal notation into RGB notation.
+ *
+ * @param {String} hexadecimal A color in hexadecimal notation.
+ * @returns {Array} `null` if the input color is not in hexadecimal notation. Otherwise, the input color as RGB value.
+ */
+function hexadecimalToRGB(hexadecimal) {
+    const components = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hexadecimal);
+
+    if (components !== null) {
+        return _.map(components.slice(1), component => parseInt(component, 16));
+    }
+
+    return null;
+}
+
 /**
  * Returns a background color with opacity style
  *
@@ -210,13 +226,10 @@ function getBackgroundColorStyle(backgroundColor) {
  * @returns {Object}
  */
 function getBackgroundColorWithOpacityStyle(backgroundColor, opacity) {
-    const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(backgroundColor);
+    const result = hexadecimalToRGB(backgroundColor);
     if (result !== null) {
-        const r = parseInt(result[1], 16);
-        const g = parseInt(result[2], 16);
-        const b = parseInt(result[3], 16);
         return {
-            backgroundColor: `rgba(${r}, ${g}, ${b}, ${opacity})`,
+            backgroundColor: `rgba(${result[1]}, ${result[1]}, ${result[1]}, ${opacity})`,
         };
     }
     return {};
@@ -444,6 +457,76 @@ function getPaymentMethodMenuWidth(isSmallScreenWidth) {
     return {width: !isSmallScreenWidth ? variables.sideBarWidth - (margin * 2) : undefined};
 }

+/**
+ * Approximates a color in RGBA notation with an equivalent color in RGB notation.
+ *
+ * @param {Array} foregroundRGB The three components of the foreground color in RGB notation.
+ * @param {Array} backgroundRGB The three components of the background color in RGB notation.
+ * @param {number} opacity The desired opacity of the foreground color.
+ * @returns {Array} A color in RGB notation that approximates the foreground color with the given opacity on top of the background color.
+ */
+function approximateRGBAWithRGB(foregroundRGB, backgroundRGB, opacity) {
+    const [foregroundR, foregroundG, foregroundB] = foregroundRGB;
+    const [backgroundR, backgroundG, backgroundB] = backgroundRGB;
+
+    return [
+        ((1 - opacity) * backgroundR) + (opacity * foregroundR),
+        ((1 - opacity) * backgroundG) + (opacity * foregroundG),
+        ((1 - opacity) * backgroundB) + (opacity * foregroundB),
+    ];
+}
+
+/**
+ * Denormalizes the three components of a color in RGB notation.
+ *
+ * @param {number} r The first normalized component of a color in RGB notation.
+ * @param {number} g The second normalized component of a color in RGB notation.
+ * @param {number} b The third normalized component of a color in RGB notation.
+ * @returns {Array} An array with the three denormalized components of a color in RGB notation.
+ */
+function denormalizeRGB(r, g, b) {
+    return [Math.floor(r * 255), Math.floor(g * 255), Math.floor(b * 255)];
+}
+
+/**
+ * Normalizes the three components of a color in RGB notation.
+ *
+ * @param {number} r The first denormalized component of a color in RGB notation.
+ * @param {number} g The second denormalized component of a color in RGB notation.
+ * @param {number} b The third denormalized component of a color in RGB notation.
+ * @returns {Array} An array with the three normalized components of a color in RGB notation.
+ */
+function normalizeRGB(r, g, b) {
+    return [r / 255, g / 255, b / 255];
+}
+
+/**
+ * Approximates the theme color for a modal based on the app's background color,
+ * the modal's backdrop, and the backdrop's opacity.
+ *
+ * @returns {String} The theme color as an RGB value.
+ */
+function getThemeColor() {
+    const backdropOpacity = variables.modalFullscreenBackdropOpacity;
+
+    const [backgroundR, backgroundG, backgroundB] = hexadecimalToRGB(themeColors.appBG);
+    const [backdropR, backdropG, backdropB] = hexadecimalToRGB(themeColors.modalBackdrop);
+    const normalizedBackdropRGB = normalizeRGB(backdropR, backdropG, backdropB);
+    const normalizedBackgroundRGB = normalizeRGB(
+        backgroundR,
+        backgroundG,
+        backgroundB,
+    );
+    const approximatedThemeRGBNormalized = approximateRGBAWithRGB(
+        normalizedBackdropRGB,
+        normalizedBackgroundRGB,
+        backdropOpacity,
+    );
+    const approximatedThemeRGB = denormalizeRGB(...approximatedThemeRGBNormalized);
+
+    return `rgb(${approximatedThemeRGB.join(', ')})`;
+}
+
 /**
  * Parse styleParam and return Styles array
  * @param {Object|Object[]} styleParam
@@ -561,6 +644,7 @@ export {
     getMiniReportActionContextMenuWrapperStyle,
     getKeyboardShortcutsModalWidth,
     getPaymentMethodMenuWidth,
+    getThemeColor,
     parseStyleAsArray,
     combineStyles,
     getPaddingLeft,
diff --git a/src/styles/variables.js b/src/styles/variables.js
index 409c9be576..461995d4a0 100644
--- a/src/styles/variables.js
+++ b/src/styles/variables.js
@@ -42,6 +42,7 @@ export default {
     emojiLineHeight: 28,
     iouAmountTextSize: 40,
     mobileResponsiveWidthBreakpoint: 800,
+    modalFullscreenBackdropOpacity: 0.5,
     tabletResponsiveWidthBreakpoint: 1024,
     safeInsertPercentage: 0.7,
     sideBarWidth: 375,
diff --git a/web/index.html b/web/index.html
index ca117f328d..638a139d4a 100644
--- a/web/index.html
+++ b/web/index.html
@@ -4,6 +4,7 @@
     <meta charset="utf-8" />
     <title>New Expensify</title>
     <meta name="description" content="Corporate cards, reimbursements, receipt scanning, invoicing, and bill pay. One app, all free.">
+    <meta name="theme-color" content="">
     <meta property="twitter:card" content="summary">
     <meta property="twitter:site" content="@expensify">
     <meta property="og:type" content="website">
-- 
2.23.0
JmillsExpensify commented 1 year ago

@eVoloshchak Can we prioritize reviewing this proposal and help keep this moving forward? Thank you!

eVoloshchak commented 1 year ago

Rather, I have now moved the web-specific calls to Modal.

This is a perfect solution!

Nice job working on this one, it's an interesting issue

A couple of things to include when opening a PR:

@dnlfrst's proposal looks good to me πŸŽ€πŸ‘€πŸŽ€ C+ reviewed! cc: @tgolen

tgolen commented 1 year ago

🟒 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.

JmillsExpensify commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

πŸ“£ @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 commented 1 year ago

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

sakluger commented 1 year ago

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

dnlfrst commented 1 year ago

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 commented 1 year ago

🟒 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 commented 1 year ago

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

sakluger commented 1 year ago

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 commented 1 year ago

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

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

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

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

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

melvin-bot[bot] commented 1 year ago

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:

sakluger commented 1 year ago

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 commented 1 year ago

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

sakluger commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 πŸ‘Œ