[HOLD for payment 2023-05-03] [$1000] Auto Complete Address, blue border doesn't cover the edge

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open any workspace, go to Connect Bank Account Manually step 2 (Company Information page).
2. Focus on Company address field, enter some address to make Auto Complete Address list visible, press Tab to make blue border appears.
3. Notice that blue border doesn't cover the edge of the dropdown.

Expected Result

Blue border should fully cover the edge dropdown.

Actual Result

Blue border doesn't cover the edge of the dropdown.

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.2.98-2
Reproducible in staging?:
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-04-11.at.14.07.01.mov

Recording.203.mp4

Expensify/Expensify Issue URL:
Issue reported by: @hungvu193
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681197151185779

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013f1335a90efc1a5f
• Upwork Job ID: 1646054336619749376
• Last Price Increase: 2023-04-12

[MelvinBot]

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

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[kadiealexander]

Reproed!

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~013f1335a90efc1a5f

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[aman-atg]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Auto Complete Address, blue border doesn't cover the edge

What is the root cause of that problem?

The rounded borders of the dropdown container are causing the outline to be truncated as we haven't applied any vertical padding for the list view to avoid it (or have border radius defined for first and last rows)

What changes do you think we should make in order to solve the problem?

We just need to add some vertical spacing to it here like styles.pv1 or styles.pv2

App/src/components/AddressSearch.js

Line 268 in 3abe7b2

styles.borderRight,

eg.

                        listView: [
                            !displayListViewBorder && styles.googleListView,
                            displayListViewBorder && styles.borderTopRounded,
                            displayListViewBorder && styles.borderBottomRounded,
                            displayListViewBorder && styles.mt1,
                            displayListViewBorder && styles.pv1,

Result after applying fix:

[akinwale]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Blue border indicating focus isn't properly rendered in the autocomplete address dropdown list.

What is the root cause of that problem?

The style which is used to indicate focus of an element makes use of a blue inset in CSS.
https://github.com/Expensify/App/blob/main/web/index.html#L45-L47

The top and bottom edges of the address dropdown have rounded corners which end up cutting off the inset, since CSS insets cannot have a border radius set.

What changes do you think we should make in order to solve the problem?

This is dependent on the overall design language, but removing the rounded corners fixes the problem as shown in the image below.

What alternative solutions did you explore? (Optional)

Using an image or using the CSS border-image-slice directive to indicate focus such that the focus indicator is rendered properly with rounded corners.

[daraksha-dk]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Auto Complete Address, blue border doesn't cover the edge

What is the root cause of that problem?

This is happening because we're not applying any padding for list inside AddressSearch.js

What changes do you think we should make in order to solve the problem?

To fix this, we should apply "12px" or "8px" of vertical padding inside AddressSearch.js for the list view

[alitoshmatov]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Auto Complete Address, blue border doesn't cover the edge

What is the root cause of that problem?

This is caused by rounded borders in dropdown container. Thus clipping outline(box-shadow is used here)

What changes do you think we should make in order to solve the problem?

We should apply top rounded corners for first child and bottom rounded corners to last child. Currently https://github.com/FaridSafi/react-native-google-places-autocomplete package which is used here doesn't allow applying individual styles for first or last element. It only allows to apply general same style for all row elements.

The best solution would be creating an issue and offering a pr request for this package to support individual styling for row elements. We can modify this components style.row property to also take functions. When applying styles to row elements it can pass an index and run this function to get current row style.

What alternative solutions did you explore? (Optional)

none

[huzaifa-99]

Proposal

Please re-state the problem that we are trying to solve in this issue.

We want the blue focus border in auto-complete address dropdown to be rounded like its container. Only the first and the last row needs to have rounded radius

What is the root cause of that problem?

We are not adding border radius to the address dropdown rows here.

What changes do you think we should make in order to solve the problem?

We need to add border top left/right radius to the first and border bottom left/right radius to the last row of the dropdown list. For this we would need to check the row index and apply styles, atm this is not supported by the react-native-google-places-autocomplete.

We would need to fix it upstream, I would suggest we update our own fork of react-native-google-places-autocomplete. For this we would need to do these

1. Move this style from View to the parent Pressable styles
2. Pass the length of total rows to the _renderRow function from this line, like this (3rd argument)

renderItem={({ item, index }) => _renderRow(item, index, dataSource.length - 1)}

and receive it in the function args like this

const _renderRow = (rowData = {}, index, totalRows) => {

3. We need to accept the styles.row as a function param (we can keep accepting it as an object) (just to recall we move the applied styles to Pressble, see step 1). When it's a function, then we would pass 2 arguments to it that would tell if it's the first row or the last row, like this

typeof props.styles.row === 'function' ? props.styles.row(index === 0, index === totalRows) : props.styles.row,

4. And finally in AddressSearch component we would use it like this here

row: (isFirst, isLast) => ([
    styles.pv4,
    styles.ph3,
    styles.overflowAuto,
    isFirst && { borderTopLeftRadius: 8, borderTopRightRadius: 8, },
    isLast && { borderBottomLeftRadius: 8, borderBottomRightRadius: 8, } 
])

Demo

Screen.Recording.2023-04-12.at.2.40.58.PM.mov

What alternative solutions did you explore? (Optional)

N/A

[mfirz]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Auto Complete Address, blue border doesn't cover the edge.

What is the root cause of that problem?

The react-native-google-places-autocomplete package lacks a way to style the first and last child listItem.

What changes do you think we should make in order to solve the problem?

To address the issue, I propose a simple solution using CSS. The container element of react-native-google-places-autocomplete package has an id attribute called result-list-id. We can directly modify the style of the first and last child of listItem using the following CSS:

#result-list-id > div > div:first-child > div > div > div {
  border-top-left-radius: 8px;
  border-top-right-radius: 8px;
}

#result-list-id > div > div:nth-last-child(2) > div > div > div {
  border-bottom-left-radius: 8px;
  border-bottom-right-radius: 8px;
}

This will add border radius to the top corners of the first child listItem and the bottom corners of the last child listItem.

What alternative solutions did you explore? (Optional)

N/A

Results

solution.mov

[MelvinBot]

📣 @firzx77! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[mfirz]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01bd5290487267fbfd

[MelvinBot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[webexpert889]

1. Please re-state the problem that we are trying to solve in this issue.
   Ans : We need to fix the blue focus border in address dropdown to be rounded around it's container. The first and the last container is having edges not focused in Blue border.

2. What is the root cause of that problem?

Ans: The root cause of this problem is that the container is placed inside a list view and due to this the list border is being overwritten by the default css of the device in which the issue is occurring (Mac, Safari, Chrome).

3. What changes do you think we should make in order to solve the problem?
   Ans: We need to add remove the inner radius of the list which is overwriting the css of the blue border. There are several ways through which we can solve the problem. Below are ways to fix this problem;

a. We need to give border radius to the list first child and last child. This will help it to adjust according to the browser default functionality and will be able to solve the problem universally.
OR
b. We can give padding to the list but this will not be suitable as per the coding standards.

Thus, the problem should be solved with the first method.

[MelvinBot]

📣 @webexpert889! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[s77rt]

@aman-atg Thanks for the proposal. I don't think your RCA is correct.

[s77rt]

@akinwale Thanks for the proposal. I don't think your RCA is correct, the use of inset box shadow is a good thing here. Otherwise the box shadow won't be visible at all.

[s77rt]

@daraksha-dk Thanks for the proposal but it's a duplicate.

[akinwale]

@akinwale Thanks for the proposal. I don't think your RCA is correct, the use of inset box shadow is a good thing here. Otherwise the box shadow won't be visible at all.

Thank you for your response. Could you please explain further? I'm not recommending removing the inset box shadow here. The box shadow will always be cropped by a rounded border, as can be shown. When you remove the rounded corners, the inset box shadow displays properly for the first and last elements (refer to the image I attached).

[mountiny]

there was a regression from this Pr so adjusting the title to wait for payment until the fix is impelmented.

[MelvinBot]

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

[MelvinBot]

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

• fix 17664: use googleEmptyListViewHeight variable with correct value #17832

If no regressions arise, payment will be issued on 2023-05-03. 🎊

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]

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:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR: [N6.1] Use Google AddressSearch component in VBA Company Step #5643
• [@s77rt] 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: [N6.1] Use Google AddressSearch component in VBA Company Step #5643 (comment)
• [@s77rt] 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: https://expensify.slack.com/archives/C049HHMV9SM/p1681981488202219
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] 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. [HOLD for payment 2023-05-03] [$1000] Auto Complete Address, blue border doesn't cover the edge #17277 (comment)
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/281307

[s77rt]

@MelvinBot I think you are confused. That PR did not fix this issue.

This issue was fixed by #17403 and the checklist is already provided here #17277 (comment)

[kadiealexander]

Sorry, just getting my ducks in a row for payment. Was there a regression from this PR as per #17277 (comment)?

[aman-atg]

@kadiealexander Yes, this is the regression that it caused - #17664

[kadiealexander]

Awesome, thanks @aman-atg!

[kadiealexander]

@aman-atg, @s77rt, @hungvu193 please apply here so I can issue payment: https://www.upwork.com/jobs/~013f1335a90efc1a5f

[hungvu193]

Thanks @kadiealexander , I've just applied

[aman-atg]

Thanks @kadiealexander, I've also applied.

[s77rt]

@kadiealexander Applied

[kadiealexander]

Thanks guys! I've sent contracts to @aman-atg and @s77rt (sorry, yours is reduced due to the regression penalty) and made payment to @hungvu193.

[s77rt]

@kadiealexander Yes, sorry I forgot to mention. I have accepted the offer.

[aman-atg]

@kadiealexander Have accepted the offer, thanks!

[kadiealexander]

Everyone is paid and the checklist is complete!! Thanks for the help, team.