[HOLD for payment 2023-05-16] [$1000] Green dot (20x20) is considerably bigger then pin icon (16x16) in LHN and not consistent in size with other pages #17163

kavimuru · 2023-04-07T22:21:34Z

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
Observe size of green dot and size of pin icon on reports archive
Open settings and observe size of green dot
Open profile and observe size of green dot
In profile->contact method-> phone number, observe size of green dot

Expected Result:

App should maintain same size for green dot SVG throughout and app should use 20x20 size for green dot on reports archive to ensure it looks right besides pin icon

Actual Result:

App uses 20x20 size for green dot besides 16x16 icon of pin and leaving reports archive and phone number page in contact method, app uses 16x16 size for green dot icon throughout all the pages

Workaround:

unknown

Platforms:

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

Version Number: 1.2.96-4
Reproducible in staging?: y
Reproducible in production?: y
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

green.dot.inconsistency.issue.mp4

Recording.164.mp4

Recording.165.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680859279508339

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b604439e23576a0b
Upwork Job ID: 1648046942074019840
Last Price Increase: 2023-05-01

MelvinBot · 2023-04-07T22:21:40Z

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

MelvinBot · 2023-04-07T22:21:45Z

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

sonialiap · 2023-04-09T17:12:10Z

The dot is indeed larger than in other instances of the dot

MelvinBot · 2023-04-09T17:12:18Z

Triggered auto assignment to @joelbettner (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

MelvinBot · 2023-04-13T10:06:01Z

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

MelvinBot · 2023-04-17T10:08:22Z

@joelbettner, @sonialiap 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

MelvinBot · 2023-04-17T19:33:10Z

Job added to Upwork: ~~https://www.upwork.com/jobs/~01b604439e23576a0b~~
https://www.upwork.com/jobs/~01567cda7bfabcebb0

MelvinBot · 2023-04-17T19:33:15Z

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

joelbettner · 2023-04-17T19:33:16Z

This is all front end and can be worked on externally.

MelvinBot · 2023-04-17T19:33:25Z

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

Prince-Mendiratta · 2023-04-17T19:33:31Z

Proposal

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

In this issue, we can see that the dot indicator on the LHN is bigger than other similar icons on the App.

What is the root cause of that problem?

This green dot stands for a pending IOU transaction. The height and width of the icon are not set for this icon, leading it to default to a bigger size.

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

As per the suggestion here, we need to set the other icons as 20x20 instead. We need to remove the height and width in these places:

OptionRowLHN
- App/src/components/LHNOptionsList/OptionRowLHN.js
  
  Line 233 in c792447
  
  <Icon src={Expensicons.Pin} height={16} width={16} />
- App/src/components/LHNOptionsList/OptionRowLHN.js
  
  Line 224 in c792447
  
  <Icon src={Expensicons.Pencil} height={16} width={16} />
- App/src/components/LHNOptionsList/OptionRowLHN.js
  
  Lines 208 to 209 in c792447
  
  height={variables.iconSizeSmall}
  
  width={variables.iconSizeSmall}
OptionRow
- App/src/components/OptionRow.js
  
  Lines 266 to 267 in 8864416
  
  height={variables.iconSizeSmall}
  
  width={variables.iconSizeSmall}
- App/src/components/OptionRow.js
  
  Lines 282 to 283 in 8864416
  
  height={16}
  
  width={16}
HeaderView
- App/src/pages/home/HeaderView.js
  
  Lines 156 to 157 in c792447
  
  height={variables.iconSizeSmall}
  
  width={variables.iconSizeSmall}

EDIT
With additional details from here, we need to make the following modifications to the Icons:

Remove height and width from Icon

MenuItem
- App/src/components/MenuItem.js
  
  Line 195 in 5a3cbd8
  
  <Icon

DotIndicator

App/src/components/DotIndicatorMessage.js

Line 57 in e7cfb99

    
           <Icon src={Expensicons.DotIndicator} fill={props.type === 'error' ? colors.red : colors.green} height={variables.iconSizeSmall} width={variables.iconSizeSmall} />

What alternative solutions did you explore? (Optional)

We can resolve #17226 here as well by adding additionalStyles={[styles.ml2]} to the DotIndicator icon.

PrashantMangukiya · 2023-04-17T19:33:31Z

Proposal

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

Green dot (20x20) is considerably bigger then pin icon (16x16) in LHN and not consistent in size with other pages #17163

What is the root cause of that problem?

We are using DotIndicator from here, its default size is 20X20

App/src/components/Icon/Expensicons.js

Line 33 in 3abe7b2

import DotIndicator from '../../../assets/images/dot-indicator.svg';

Within ContactMethodDetailsPage.js we are displaying green icon via line 175:

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.js

Line 175 in 3abe7b2

Within OptionRowLHN.js we are displaying green icon via line 227:

App/src/components/LHNOptionsList/OptionRowLHN.js

Line 227 in 3abe7b2

    
           {optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner && <Icon src={Expensicons.DotIndicator} fill={colors.green} />}

We can see at both places i.e. Contact Method Details Page and Option Row LHN height and width prop not set, so it will take default width height i.e. 20x20.

This is the root cause of the problem i.e. Both places it shows 20x20 icon and other place it shows small icon.

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

Within ContactMethodDetailsPage.js set icon width/height as variables.iconSizeSmall (i.e. 16) as shown below:

// <Icon src={Expensicons.DotIndicator} fill={colors.green} />  // *** OLD CODE
<Icon src={Expensicons.DotIndicator} fill={colors.green} height={variables.iconSizeSmall} width={variables.iconSizeSmall} />. // *** Updated Code

Within OptionRowLHN.js set icon width/height as variables.iconSizeSmall (i.e. 16) as shown below:

// {optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner && <Icon src={Expensicons.DotIndicator} fill={colors.green} />}. // *** OLD CODE

// Updated Code
{optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner && (
    <Icon
        src={Expensicons.DotIndicator}
        fill={colors.green}
        height={variables.iconSizeSmall}
        width={variables.iconSizeSmall}
    />
)}

So this will solve the problem as shown in results.

What alternative solutions did you explore? (Optional)

None

Results

Before	After

MelvinBot · 2023-04-17T19:33:32Z

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

aman-atg · 2023-04-17T19:33:34Z

Proposal

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

Green dot is bigger in size & doesn't have any margin applied to it in LHN

What is the root cause of that problem?

The Dot Icon does not have proper width and height defined, and it also lacks a container that includes a margin-left property.

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

To fix this, we just need to wrap the Icon in the View component with styles.ml2 inside OptionRowLHN.js here like below:

                            {optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner && (
                                <View style={styles.ml2}>
                                    <Icon src={Expensicons.DotIndicator} fill={colors.green} />
                                </View>
                            )}

Additionally, as per latest requirement we need to update Icons with 20x20 size throughout our app wherever 16×16 is being used

This will fix both #17163 & #17226

What alternative solutions did you explore? (Optional)

We can get the same results by explicitly defining width and height for these icons so that it's easier to update the sizes in future

Pujan92 · 2023-04-17T19:33:36Z

Proposal

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

Green/Red dot size is inconsistent in some places(LHN, Contact method details page, Form help message)

What is the root cause of that problem?

We are not providing the height & width explicitly of 16px which we are giving at other places.

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

Apply height={variables.iconSizeSmall} width={variables.iconSizeSmall} to the places where we have used the DotIndicator Icon.

App/src/components/LHNOptionsList/OptionRowLHN.js

Line 227 in 8b570f6

    
           {optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner && <Icon src={Expensicons.DotIndicator} fill={colors.green} />}

App/src/components/FormHelpMessage.js

Line 40 in 8b570f6

{props.isError && <Icon src={Expensicons.DotIndicator} fill={colors.red} />}

App/src/pages/settings/Profile/Contacts/ContactMethodDetailsPage.js

Line 175 in 8b570f6

allroundexperts · 2023-04-18T08:33:43Z

Proposal

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

Green dot is considerably bigger then pin icon in LHN. It's also not consistent on other pages.

What is the root cause of that problem?

The root cause is that at some places we're setting the height and width of the icon and at other places, we're not setting it. This causes the inconsistency.

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

To make the icon consistent with other icons in the LHN, we need to change this to:

{optionItem.hasOutstandingIOU && !optionItem.isIOUReportOwner && <Icon  src={Expensicons.DotIndicator}  fill={colors.green}  height={variables.iconSizeSmall} width={variables.iconSizeSmall}  />}

As a small refactor, some icons here are using the size explicitly ie width={16px} instead of using variables.iconSizeSmall. So we can replace those values with variables.iconSizeSmall as well.

At other places, besides the LHN, there is a mix of sizes of this icon being used. We can either convert those all to 20px or 16px. @shawnborton Can confirm the size as well as if the size should be the same on all other sizes. For reference, we're using this component in the following components:

DotIndicatorMessage -> 16px

FormHelpMessage -> 20px

MenuItem -> 20px (Other icons here are 20px as well so I think this is consistent)

OptionRow -> 16px (Other icons here are 16px as well so I think this is consistent)

HeaderView -> 16px (Other icons here seem to be using 20px so this is inconsistent)

ContactMethodDetailsPage -> 20px (Other icons here are using 20px as well so this is consistent)

What alternative solutions did you explore? (Optional)

None

shawnborton · 2023-04-18T13:04:09Z

Hmm, let's try making all of our icons 20x20 and then seeing how it goes from there? We can still use a 20x20 dot indicator icon but maybe we can reduce the shape of the actual ellipse inside of it.

So that would include seeing what an updated pin icon in the LHN rows look like at 20x20 as well.

Prince-Mendiratta · 2023-04-18T13:10:33Z

@shawnborton this is how it looks on the LHN with updated icon:

MelvinBot · 2023-05-01T18:21:15Z

📣 @Prince-Mendiratta You have been assigned to this job by @johncschuster!
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 📖

johncschuster · 2023-05-01T18:21:31Z

Assigned!

Prince-Mendiratta · 2023-05-01T18:28:29Z

PR is up for review!

Applied in Upwork.

cc @aimane-chnaif @johncschuster

johncschuster · 2023-05-01T19:03:39Z

Thanks for the quick reply, @Prince-Mendiratta! I've just sent you the offer!

melvin-bot · 2023-05-08T20:08:42Z

@johncschuster, @joelbettner, @sonialiap, @Prince-Mendiratta, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2023-05-09T03:38:22Z

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

melvin-bot · 2023-05-09T03:38:22Z

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

[Icon Size] GBR, RBR & related icon size 20x20 #18242

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

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

melvin-bot · 2023-05-09T03:38:24Z

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:

[@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
[@aimane-chnaif] 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:
[@aimane-chnaif] 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:
[@aimane-chnaif] Determine if we should create a regression test for this bug.
[@aimane-chnaif] 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.
[@johncschuster / @sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Prince-Mendiratta · 2023-05-10T06:14:02Z

Sharing the timeline of this issue to help with the eventual analysis, calculated with this tool.

🐛 Issue created at: Fri, 07 Apr 2023 22:21:34 GMT
🧯 Help Wanted at: Mon, 17 Apr 2023 19:33:30 GMT
🕵🏻 Contributor assigned at: Mon, 01 May 2023 18:21:15 GMT
🛸 PR created at: Mon, 01 May 2023 18:27:27 GMT
🎯 PR merged at: Tue, 02 May 2023 18:32:36 GMT
⌛ Business Days Elapsed between assignment and PR merge: 2

joelbettner · 2023-05-16T19:37:57Z

@aimane-chnaif what is the status here? The PR got merged. Can this be closed?

aimane-chnaif · 2023-05-16T19:42:58Z

payment is remaining

joelbettner · 2023-05-16T20:28:07Z

@aimane-chnaif what about the checklists above?

sonialiap · 2023-05-17T14:31:32Z

@dhanashree-sawant offer sent for reporting - paid
@aimane-chnaif offer sent (+ 50% merge bonus) - paid
@Prince-Mendiratta offer sent (+ 50% merge bonus) - paid

aimane-chnaif · 2023-05-17T14:36:18Z

@sonialiap offer expired

aimane-chnaif · 2023-05-17T14:51:42Z

For BugZero Checklist:

It's tough to find which PR caused regression. It's inconsistency issue but more like polish.

Regression Test Proposal

@luacmartins how about adding some more verification in #17912 (comment)?

3 - Verify the Pin icon is showing in the LHN and the size is 20x20.
10 - Verify that GBR and RBR are of the same size as 20x20.
11 - Type any message in composer and verify that draft icon shows with the size of 20x20.

luacmartins · 2023-05-17T14:54:15Z

TBH I don't think we need a regression test for this.

sonialiap · 2023-05-17T14:54:42Z

Wow, what timing with the Upwork job expiring 😆 . I made a new Upwork job and sent an offer @aimane-chnaif

sonialiap · 2023-05-17T14:58:23Z

All paid out. Since we're not doing a regression test for this. Closing

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 7, 2023

melvin-bot bot assigned sonialiap Apr 7, 2023

sonialiap added the Engineering label Apr 9, 2023

melvin-bot bot assigned joelbettner Apr 9, 2023

melvin-bot bot added the Overdue label Apr 12, 2023

joelbettner added the External Added to denote the issue can be worked on by a contributor label Apr 17, 2023

melvin-bot bot changed the title ~~Green dot (20x20) is considerably bigger then pin icon (16x16) in LHN and not consistent in size with other pages~~ [$1000] Green dot (20x20) is considerably bigger then pin icon (16x16) in LHN and not consistent in size with other pages Apr 17, 2023

melvin-bot bot assigned johncschuster Apr 17, 2023

melvin-bot bot removed the Overdue label Apr 17, 2023

melvin-bot bot assigned aimane-chnaif Apr 17, 2023

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

johncschuster assigned Prince-Mendiratta May 1, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label May 1, 2023

Prince-Mendiratta mentioned this issue May 1, 2023

[Icon Size] GBR, RBR & related icon size 20x20 #18242

Merged

57 tasks

melvin-bot bot added the Reviewing Has a PR in review label May 1, 2023

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

melvin-bot bot removed the Reviewing Has a PR in review label May 9, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 15, 2023

sonialiap closed this as completed May 17, 2023

[HOLD for payment 2023-05-16] [$1000] Green dot (20x20) is considerably bigger then pin icon (16x16) in LHN and not consistent in size with other pages #17163

[HOLD for payment 2023-05-16] [$1000] Green dot (20x20) is considerably bigger then pin icon (16x16) in LHN and not consistent in size with other pages #17163

Comments

kavimuru commented Apr 7, 2023 • edited by joelbettner Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

MelvinBot commented Apr 7, 2023

MelvinBot commented Apr 7, 2023 • edited by sonialiap Loading

Bug0 Triage Checklist (Main S/O)

sonialiap commented Apr 9, 2023

MelvinBot commented Apr 9, 2023

MelvinBot commented Apr 13, 2023

MelvinBot commented Apr 17, 2023

MelvinBot commented Apr 17, 2023 • edited by sonialiap Loading

MelvinBot commented Apr 17, 2023

joelbettner commented Apr 17, 2023

MelvinBot commented Apr 17, 2023

Prince-Mendiratta commented Apr 17, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

PrashantMangukiya commented Apr 17, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

MelvinBot commented Apr 17, 2023

aman-atg commented Apr 17, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Pujan92 commented Apr 17, 2023

Proposal

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

What is the root cause of that problem?

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

allroundexperts commented Apr 18, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

shawnborton commented Apr 18, 2023

Prince-Mendiratta commented Apr 18, 2023

MelvinBot commented May 1, 2023

johncschuster commented May 1, 2023

Prince-Mendiratta commented May 1, 2023

johncschuster commented May 1, 2023

melvin-bot bot commented May 8, 2023

melvin-bot bot commented May 9, 2023

melvin-bot bot commented May 9, 2023

melvin-bot bot commented May 9, 2023 • edited by sonialiap Loading

Prince-Mendiratta commented May 10, 2023

joelbettner commented May 16, 2023

aimane-chnaif commented May 16, 2023

joelbettner commented May 16, 2023

sonialiap commented May 17, 2023 • edited Loading

aimane-chnaif commented May 17, 2023

aimane-chnaif commented May 17, 2023

luacmartins commented May 17, 2023

sonialiap commented May 17, 2023

sonialiap commented May 17, 2023

kavimuru commented Apr 7, 2023 •

edited by joelbettner

Loading

MelvinBot commented Apr 7, 2023 •

edited by sonialiap

Loading

MelvinBot commented Apr 17, 2023 •

edited by sonialiap

Loading

Prince-Mendiratta commented Apr 17, 2023 •

edited

Loading

aman-atg commented Apr 17, 2023 •

edited

Loading

melvin-bot bot commented May 9, 2023 •

edited by sonialiap

Loading

sonialiap commented May 17, 2023 •

edited

Loading