[HOLD for payment 2023-02-14] [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode #13838

kavimuru · 2022-12-27T15:54:19Z

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:

Open the app
Go to any workspace
Choose Connect Bank Account
Choose Connect manually
Go to step 2 of the Form
Write any value in company address input to get items
Hold any item and you'll notice the background color isn't according to the theme

Expected Result:

Background and opacity should be according to our theme

Actual Result:

It's not according to our theme

Workaround:

unknown

Platforms:

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

MacOS / Chrome / Safari
MacOS / Desktop

Version Number: 1.2.43-1
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:

select.issue.mp4

Expensify/Expensify Issue URL:
Issue reported by: @daraksha-dk
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1672076042941419

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01e12ff05603e27012
Upwork Job ID: 1618651782887931904
Last Price Increase: 2023-01-26

slafortune · 2023-01-23T23:08:57Z

@grgia Do you have any update on this? I'm going to move it back to daily so we can push it forward and not lose sight of it 👍

melvin-bot · 2023-01-26T16:47:15Z

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

melvin-bot · 2023-01-26T16:47:19Z

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

melvin-bot · 2023-01-26T16:47:26Z

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

melvin-bot · 2023-01-26T16:47:29Z

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

daraksha-dk · 2023-01-26T16:47:40Z

Proposal

There is no style prop to change the background color when row is being pressed, the solution is to set the backgroundColor

To fix this we need these changes:

 diff --git a/src/components/AddressSearch.js b/src/components/AddressSearch.js
index 4d465c990..8547c6fa7 100644
--- a/src/components/AddressSearch.js
+++ b/src/components/AddressSearch.js
@@ -10,6 +10,8 @@ import styles from '../styles/styles';
 import TextInput from './TextInput';
 import Log from '../libs/Log';
 import * as GooglePlacesUtils from '../libs/GooglePlacesUtils';
+import themeColors from '../styles/themes/default';
+import * as StyleUtils from '../styles/StyleUtils';
 
 // The error that's being thrown below will be ignored until we fork the
 // react-native-google-places-autocomplete repo and replace the
@@ -220,6 +222,7 @@ const AddressSearch = (props) => {
                             styles.borderRight,
                         ],
                         row: [
+                            StyleUtils.getBackgroundColorStyle(themeColors.componentBG),
                             styles.pv4,
                             styles.ph3,
                             styles.overflowAuto,

Demo

company-address.mp4

Pujan92 · 2023-01-26T16:48:12Z

Proposal

Use listUnderlayColor property to provide background color when the option gets pressed.

iff --git a/src/libs/actions/Policy.js b/src/libs/actions/Policy.js
index 0f6784e7d..038d8e941 100644
pujanshah@Pujans-MacBook-Air App % 
pujanshah@Pujans-MacBook-Air App % git diff
diff --git a/src/components/AddressSearch.js b/src/components/AddressSearch.js
index 1b749cc72..c2646d1b1 100644
                         description: [styles.googleSearchText],
                         separator: [styles.googleSearchSeparator],
                     }}
+                    listUnderlayColor={themeColors.border}
                     onLayout={(event) => {

situchan · 2023-01-26T17:08:38Z

Proposal

Item pressed color should be consistent across the app

App/src/styles/StyleUtils.js

Lines 273 to 274 in fafab61

    
           case CONST.BUTTON_STATES.PRESSED: 
        
               return {backgroundColor: themeColors.buttonPressedBG};

Solution:
Pass listUnderlayColor value to this component:

App/src/components/AddressSearch.js

Line 154 in fafab61

<GooglePlacesAutocomplete

+ import themeColors from '../styles/themes/default';

+                   listUnderlayColor={themeColors.buttonPressedBG}

pressed.color.mov

NOTE: themeColors.border is hover color, not pressed color

jatinsonijs · 2023-01-26T17:15:39Z

Root Case
react-native-google-places-autocomplete use TouchableHighlight for items onPress and default underlay color value is '#c8c7cc'.
https://github.com/FaridSafi/react-native-google-places-autocomplete/blob/fb9d70b04cb6403348bd030816bccac2f534315d/GooglePlacesAutocomplete.js#L633

Proposal
We have to override this underlay color value.
I think we should just use transparent as underlay colour, It will work like other list item, on press opacity will reduce

diff --git a/src/components/AddressSearch.js b/src/components/AddressSearch.js
index 4d465c990a..bf3de142a4 100644
--- a/src/components/AddressSearch.js
+++ b/src/components/AddressSearch.js
@@ -10,6 +10,7 @@ import styles from '../styles/styles';
 import TextInput from './TextInput';
 import Log from '../libs/Log';
 import * as GooglePlacesUtils from '../libs/GooglePlacesUtils';
+import themeColors from '../styles/themes/default';
 
 // The error that's being thrown below will be ignored until we fork the
 // react-native-google-places-autocomplete repo and replace the
@@ -208,6 +209,7 @@ const AddressSearch = (props) => {
                             }
                         },
                     }}
+                    listUnderlayColor={themeColors.transparent}
                     styles={{
                         textInputContainer: [styles.flexColumn],
                         listView: [

Pujan92 · 2023-01-26T17:26:03Z

Color I have used based on this slack message

jatinsonijs · 2023-01-26T17:43:24Z

Actually I have used transparent based on OptionRow on press behaviour on native, we don't have hover effect on mobile and for web this package not support hover color so I have used transparent to match OptionRow onPress behaviour.

situchan · 2023-02-01T08:12:58Z

@Santhosh-Sellavel @grgia both PRs are ready for review
App PR: #14724
forked repo PR: Expensify/react-native-google-places-autocomplete#2
Test step is in app PR description

grgia · 2023-02-06T17:30:21Z

@slafortune Just a heads up, this issue qualifies for the speed bonus as everything was completed in 2 days, thanks!

MelvinBot · 2023-02-07T17:38:11Z

Reviewing label has been removed, please complete the "BugZero Checklist".

MelvinBot · 2023-02-07T17:38:13Z

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

Fix: Address suggestion lists hover and selection color issue #14724

If no regressions arise, payment will be issued on 2023-02-14. 🎊

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

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

MelvinBot · 2023-02-07T17:38:16Z

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 / @grgia] The PR that introduced the bug has been identified. Link to the PR:
[@Santhosh-Sellavel / @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 / @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:
[@situchan] Propose regression test steps to ensure the same bug will not reach production again.
[@slafortune] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Santhosh-Sellavel · 2023-02-07T17:40:23Z

@situchan please propose regression test steps.

situchan · 2023-02-07T19:54:33Z

Regression Test Proposal

Open the app and login with any account
Go to Settings -> Workspaces -> select any workspace
Choose "Connect bank account" -> "Connect manually"
Go to Step 2 of the Form
Write any value in "Company address" input to get suggestions
Hover any item and verify that background color is #1A3D32 which matches our dark theme
Press any item and hold to verify that background color is #467164 which matches our dark theme
Do we agree 👍 or 👎

slafortune · 2023-02-14T22:55:49Z

I've send invites to @Santhosh-Sellavel , @situchan , and @daraksha-dk

@Santhosh-Sellavel and @situchan can you make sure to complete the above check list? Thanks!

Santhosh-Sellavel · 2023-02-14T23:01:29Z

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 / @grgia] The PR that introduced the bug has been identified. Link to the PR: This seems to be missing part of the dark theme support.

[@Santhosh-Sellavel / @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: N/A

[@Santhosh-Sellavel / @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:
Regression test steps should be enough. To catch other bugs similar I believe we have test cases in place for dark theme support.

[@situchan] Propose regression test steps to ensure the same bug will not reach production again. [HOLD for payment 2023-02-14] [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode #13838 (comment)

[@slafortune] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Santhosh-Sellavel · 2023-02-14T23:02:09Z

@grgia Can you check and let me know if you differ here -> #13838 (comment)

slafortune · 2023-02-16T21:58:02Z

Paid all contracts 👍

@grgia can you confirm the above looks good to you? Do regression steps look good?

grgia · 2023-02-17T22:53:58Z

@slafortune looks good to me!

MelvinBot · 2023-02-21T20:31:32Z

@slafortune, @grgia, @Santhosh-Sellavel, @situchan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

grgia · 2023-02-22T19:26:21Z

@slafortune are we good to close this one out?

slafortune · 2023-02-27T21:37:37Z

I was just waiting for this convo - https://expensify.slack.com/archives/C01SKUP7QR0/p1677276599228649

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 27, 2022

melvin-bot bot locked and limited conversation to collaborators Dec 27, 2022

melvin-bot bot assigned slafortune Dec 27, 2022

grgia self-assigned this Dec 29, 2022

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jan 12, 2023

melvin-bot bot added the Overdue label Jan 20, 2023

melvin-bot bot removed the Overdue label Jan 23, 2023

slafortune added Daily KSv2 and removed Weekly KSv2 labels Jan 23, 2023

grgia added the External Added to denote the issue can be worked on by a contributor label Jan 26, 2023

melvin-bot bot unlocked this conversation Jan 26, 2023

melvin-bot bot changed the title ~~Company address : Pressing on the address suggestion opacity is background are not in the dark mode~~ [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode Jan 26, 2023

melvin-bot bot added Overdue labels Jan 26, 2023

melvin-bot bot assigned Santhosh-Sellavel Jan 26, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 26, 2023

situchan mentioned this issue Feb 1, 2023

Adding Hover and selection highlight customization support Expensify/react-native-google-places-autocomplete#2

Merged

54 tasks

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Feb 7, 2023

melvin-bot bot changed the title ~~[$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode~~ [HOLD for payment 2023-02-14] [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode Feb 7, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Feb 7, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Daily KSv2 labels Feb 13, 2023

melvin-bot bot added the Overdue label Feb 20, 2023

melvin-bot bot added Overdue and removed Overdue labels Feb 22, 2023

grgia closed this as completed Feb 25, 2023

melvin-bot bot removed the Overdue label Feb 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2023-02-14] [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode #13838

[HOLD for payment 2023-02-14] [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode #13838

kavimuru commented Dec 27, 2022 •

edited by melvin-bot bot

Loading

slafortune commented Jan 23, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

daraksha-dk commented Jan 26, 2023

Pujan92 commented Jan 26, 2023

situchan commented Jan 26, 2023 •

edited

Loading

jatinsonijs commented Jan 26, 2023 •

edited

Loading

Pujan92 commented Jan 26, 2023

jatinsonijs commented Jan 26, 2023 •

edited

Loading

situchan commented Feb 1, 2023

grgia commented Feb 6, 2023

MelvinBot commented Feb 7, 2023

MelvinBot commented Feb 7, 2023

MelvinBot commented Feb 7, 2023

Santhosh-Sellavel commented Feb 7, 2023

situchan commented Feb 7, 2023

slafortune commented Feb 14, 2023

Santhosh-Sellavel commented Feb 14, 2023

Santhosh-Sellavel commented Feb 14, 2023 •

edited

Loading

slafortune commented Feb 16, 2023

grgia commented Feb 17, 2023

MelvinBot commented Feb 21, 2023

grgia commented Feb 22, 2023

slafortune commented Feb 27, 2023

[HOLD for payment 2023-02-14] [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode #13838

[HOLD for payment 2023-02-14] [$1000] Company address : Pressing on the address suggestion opacity is background are not in the dark mode #13838

Comments

kavimuru commented Dec 27, 2022 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

slafortune commented Jan 23, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

melvin-bot bot commented Jan 26, 2023

daraksha-dk commented Jan 26, 2023

Proposal

Demo

Pujan92 commented Jan 26, 2023

situchan commented Jan 26, 2023 • edited Loading

Proposal

jatinsonijs commented Jan 26, 2023 • edited Loading

Pujan92 commented Jan 26, 2023

jatinsonijs commented Jan 26, 2023 • edited Loading

situchan commented Feb 1, 2023

grgia commented Feb 6, 2023

MelvinBot commented Feb 7, 2023

MelvinBot commented Feb 7, 2023

MelvinBot commented Feb 7, 2023

Santhosh-Sellavel commented Feb 7, 2023

situchan commented Feb 7, 2023

slafortune commented Feb 14, 2023

Santhosh-Sellavel commented Feb 14, 2023

Santhosh-Sellavel commented Feb 14, 2023 • edited Loading

slafortune commented Feb 16, 2023

grgia commented Feb 17, 2023

MelvinBot commented Feb 21, 2023

grgia commented Feb 22, 2023

slafortune commented Feb 27, 2023

kavimuru commented Dec 27, 2022 •

edited by melvin-bot bot

Loading

situchan commented Jan 26, 2023 •

edited

Loading

jatinsonijs commented Jan 26, 2023 •

edited

Loading

jatinsonijs commented Jan 26, 2023 •

edited

Loading

Santhosh-Sellavel commented Feb 14, 2023 •

edited

Loading