Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hold for payment 7-17][$1000] Web - Console log "Invalid Attempt to destructure non-iterable instance" error when clicking 'Add reaction' icon #21833

Closed
2 of 6 tasks
kbecciv opened this issue Jun 28, 2023 · 41 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@kbecciv
Copy link

kbecciv commented Jun 28, 2023

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 an existing chat
  2. Open JS console
  3. Send a message
  4. Right-click on the sent message
  5. Click on 'Add reaction' icon

Expected Result:

Console log error shouldn't be displayed

Actual Result:

Console log error displayed

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: 1.3.34.1
Reproducible in staging?: n/a
Reproducible in production?: n/a
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

Screencast.from.2023-06-28.20-57-5.mp4

Expensify/Expensify Issue URL:
Issue reported by:@Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1687469634741619

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0177a9790c8518250d
  • Upwork Job ID: 1674165865126748160
  • Last Price Increase: 2023-06-28
@kbecciv kbecciv added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Jun 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 28, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 28, 2023

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

@StevenKKC
Copy link
Contributor

Proposal

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

When clicking 'Add reaction' icon, console log error is shown.

What is the root cause of that problem?

When show modal, status bar color will be set using StyleUtils.getThemeBackgroundColor.

const showModal = () => {
const statusBarColor = StatusBar.getBackgroundColor();
const isFullScreenModal =
props.type === CONST.MODAL.MODAL_TYPE.CENTERED || props.type === CONST.MODAL.MODAL_TYPE.CENTERED_UNSWIPEABLE || props.type === CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED;
setPreviousStatusBarColor(statusBarColor);
// If it is a full screen modal then match it with appBG, otherwise we use the backdrop color
setStatusBarColor(isFullScreenModal ? themeColors.appBG : StyleUtils.getThemeBackgroundColor(statusBarColor));
props.onModalShow();
};

StyleUtils.getThemeBackgroundColor will be called twice when show popup menu and when show reaction modal.
And the return value of the first call becomes the input of the second call.
The return value of the first call is of the same format as rgb(6, 33, 20).
function getThemeBackgroundColor(bgColor = themeColors.appBG) {
const backdropOpacity = variables.modalFullscreenBackdropOpacity;
const [backgroundRed, backgroundGreen, backgroundBlue] = hexadecimalToRGBArray(bgColor);
const [backdropRed, backdropGreen, backdropBlue] = hexadecimalToRGBArray(themeColors.modalBackdrop);
const normalizedBackdropRGB = convertRGBToUnitValues(backdropRed, backdropGreen, backdropBlue);
const normalizedBackgroundRGB = convertRGBToUnitValues(backgroundRed, backgroundGreen, backgroundBlue);
const themeRGBNormalized = convertRGBAToRGB(normalizedBackdropRGB, normalizedBackgroundRGB, backdropOpacity);
const themeRGB = convertUnitValuesToRGB(...themeRGBNormalized);
return `rgb(${themeRGB.join(', ')})`;
}

hexadecimalToRGBArray function can't convert this format and return undefined.

function hexadecimalToRGBArray(hexadecimal) {
const components = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hexadecimal);
if (components === null) {
return undefined;
}
return _.map(components.slice(1), (component) => parseInt(component, 16));
}

hexadecimalToRGBArray must return an array of at least three items. If it doesn't, there is no way to convert the result from hexadecimalToRGBArray into values for backgroundRed, backgroundGreen, backgroundBlue.

const [backgroundRed, backgroundGreen, backgroundBlue] = hexadecimalToRGBArray(bgColor);

So the error Invalid attempt to destructure non-iterable instance occurs.

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

StyleUtils.getThemeBackgroundColor should return in the form of #062114.
We can add rgbArrayToHexadecimal function which takes rgb array and return hex format color.
And in StyleUtils.getThemeBackgroundColor return hex format color using above function.

What alternative solutions did you explore? (Optional)

None.

@jfquevedol2198
Copy link
Contributor

jfquevedol2198 commented Jun 28, 2023

Proposal

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

Web - Console log error when clicking 'Add reaction' icon

What is the root cause of that problem?

getThemeBackgroundColor in src/styles/StyleUtils.js is designed to get hexadecimal color. but it sometimes gets rgb color.
At that time, hexadecimalToRGBArray method returns undefined. so this line throws error.

const [backgroundRed, backgroundGreen, backgroundBlue] = hexadecimalToRGBArray(bgColor);

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

function getThemeBackgroundColor(bgColor = themeColors.appBG) {

For the case that bgColor is rgb color, add rgbToArray function to get [r, g, b]] array from rgb color

function rgbToArray(rgbColor) {
    const components = /^rgb\(([0-9]{1,2}),\s*([0-9]{1,2}),\s*([0-9]{1,2})\)$/i.exec(rgbColor);
    if (components === null) return undefined;
    return components.slice(1);
}

And, update

const [backgroundRed, backgroundGreen, backgroundBlue] = hexadecimalToRGBArray(bgColor);

to

const [backgroundRed, backgroundGreen, backgroundBlue] = rgbToArray(bgColor) || hexadecimalToRGBArray(bgColor);

What alternative solutions did you explore? (Optional)

N/A

@bhargavagonugunta
Copy link

hear check out this hexadecimal values parameters are not hexadecimal
check out the image
Screenshot 2023-06-29 at 1 21 44 AM

@melvin-bot
Copy link

melvin-bot bot commented Jun 28, 2023

📣 @bhargavagonugunta! 📣
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.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@Christinadobrzyn Christinadobrzyn removed the Needs Reproduction Reproducible steps needed label Jun 28, 2023
@Christinadobrzyn
Copy link
Contributor

I can reproduce - adding External label

@Christinadobrzyn Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label Jun 28, 2023
@melvin-bot melvin-bot bot changed the title Web - Console log error when clicking 'Add reaction' icon [$1000] Web - Console log error when clicking 'Add reaction' icon Jun 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 28, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0177a9790c8518250d

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 28, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 28, 2023

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

@Christinadobrzyn Christinadobrzyn changed the title [$1000] Web - Console log error when clicking 'Add reaction' icon [$1000] Web - Console log "Invalid Attempt to destructure non-iterable instance" error when clicking 'Add reaction' icon Jun 28, 2023
@kaomoua
Copy link

kaomoua commented Jun 28, 2023

Contributor details
Your Expensify account email: kao.moua94@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/kaomoua

@melvin-bot
Copy link

melvin-bot bot commented Jun 28, 2023

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

@aliammar1995
Copy link

When I do testing, I don't see console error

@bhargavagonugunta
Copy link

bhargavagonugunta commented Jun 29, 2023 via email

@bhargavagonugunta
Copy link

bhargavagonugunta commented Jun 29, 2023 via email

@jeet-dhandha
Copy link
Contributor

jeet-dhandha commented Jun 29, 2023

Proposal

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

  • Due to some styling issue a console error is logged.

What is the root cause of that problem?

  • The root cause is the color format of StatusBar.getBackgroundColor() which is rgba(0,0,0,0). The color format should have been in hexcode.
  • StatusBarColor is then passed to getThemeBackgroundColor() which is expecting a hexcode. This is causing the error.
function getThemeBackgroundColor(bgColor = themeColors.appBG) {
    const backdropOpacity = variables.modalFullscreenBackdropOpacity;
    const [backgroundRed, backgroundGreen, backgroundBlue] = hexadecimalToRGBArray(bgColor);
    const [backdropRed, backdropGreen, backdropBlue] = hexadecimalToRGBArray(themeColors.modalBackdrop);
    const normalizedBackdropRGB = convertRGBToUnitValues(backdropRed, backdropGreen, backdropBlue);
    const normalizedBackgroundRGB = convertRGBToUnitValues(backgroundRed, backgroundGreen, backgroundBlue);
    const themeRGBNormalized = convertRGBAToRGB(normalizedBackdropRGB, normalizedBackgroundRGB, backdropOpacity);
    const themeRGB = convertUnitValuesToRGB(...themeRGBNormalized);

    return `rgb(${themeRGB.join(', ')})`;
}

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

  • Create a function that converts rgba, rgb, short-hexcode to hexcode.
Convert Function code
function convertColorToHex(color) {
  if (typeof color !== "string") {
    return null;
  }
  if (color.startsWith("rgba")) {
    // eslint-disable-next-line no-unused-vars
    const [un, r, g, b, a] = color.match(/rgba\((\d+),\s*(\d+),\s*(\d+),\s*([\d.]+)\)/);
    return `#${Number(r).toString(16).padStart(2, "0")}${Number(g).toString(16).padStart(2, "0")}${Number(b).toString(16).padStart(2, "0")}`;
  }
  if (color.startsWith("rgb")) {
    // eslint-disable-next-line no-unused-vars
    const [un, r, g, b] = color.match(/rgb\((\d+),\s*(\d+),\s*(\d+)\)/);
    return `#${Number(r).toString(16).padStart(2, "0")}${Number(g).toString(16).padStart(2, "0")}${Number(b).toString(16).padStart(2, "0")}`;
  }
  if (color.startsWith("#")) {
    const hex = color.replace("#", "");
    if (hex.length === 3) {
      return `#${hex[0]}${hex[0]}${hex[1]}${hex[1]}${hex[2]}${hex[2]}`;
    }
    if (hex.length === 6) {
      return `#${hex}`;
    }
  }
  return null;
}
  • Use the function in hexadecimalToRGBArray() to convert rgba to hexcode.
Usage code
function hexadecimalToRGBArray(hexadecimal) {
+   const color = convertColorToHex(hexadecimal);
+   if (color === null) {
+       return undefined;
+   }
-   const components = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hexadecimal);
+   const components = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(color);

    if (components === null) {
        return undefined;
    }

    return _.map(components.slice(1), (component) => parseInt(component, 16));
}

What alternative solutions did you explore? (Optional)

  • N/A

@melvin-bot

This comment was marked as outdated.

@melvin-bot
Copy link

melvin-bot bot commented Jul 21, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 25, 2023

@anmurali, @Christinadobrzyn, @Santhosh-Sellavel Still overdue 6 days?! Let's take care of this!

@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

@anmurali, @Christinadobrzyn, @Santhosh-Sellavel Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@laurenreidexpensify
Copy link
Contributor

This one fell through the cracks - apols @Natnael-Guchima, we still owe you for reporting bonus.

@Natnael-Guchima payment details are in #23914 and an Upwork offer has been sent

@Natnael-Guchima
Copy link

No problem @laurenreidexpensify

@laurenreidexpensify
Copy link
Contributor

Payment summary

@Natnael-Guchima
Copy link

@laurenreidexpensify I am not paid yet

@Nathan-Mulugeta
Copy link

Nathan-Mulugeta commented Jul 31, 2023

Hey @laurenreidexpensify I think you have paid me on upwork, I think the reporter is @Natnael-Guchima

@laurenreidexpensify
Copy link
Contributor

ah yes i just noticed this! Sorry folks Monday brain!! Nat v Nat!!

@Natnael-Guchima
Copy link

No problem 😁

@laurenreidexpensify
Copy link
Contributor

@Natnael-Guchima offer sent in Upwork 😅

@Natnael-Guchima
Copy link

Accepted the offer. Thanks @laurenreidexpensify 😁

@laurenreidexpensify
Copy link
Contributor

Okay we can close now! Correct Nat has been paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

14 participants