[HOLD for payment 2023-07-18][$1000] Dev - Console warning on pressing any mentioned user from conversation

[kbecciv]

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. Login to ND
2. Go to any conversation
3. Type a text, mention any user in that text and send it (open console)
4. Click on that particular mentioned in the chat
5. Notice on clicking mentioned it throws warning

Expected Result:

No console warning should appear

Actual Result:

Console warning appears

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.3.29-0
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

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010aec32706776696c
• Upwork Job ID: 1673995397535133696
• Last Price Increase: 2023-06-28

[melvin-bot]

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

[melvin-bot]

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

[kbecciv]

Proposal

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

• Dev - Console warning on pressing any mentioned user from conversation

What is the root cause of that problem?

• The root cause is we recently started using <TextLink> component for MentionUserRenderer.js instead of just <Text> so here the problem is we are not passing accessibilityLabel which required props that is the cause of the error as this should be mandatory as focusable is always true that required accessibilityLabel currently we are not passing or not even passing accessible to false that makes to throw warning on console

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

• We need to add accessibilityLabel in order to remove the error log this is mandatory because by default focusable is true so we do need to provide accessibilityLabel
• We need to define new accessibilityHints for the mentioned user which will be passed to accessibilityLabel message in both languages
• If we don’t need accessibilityLabel inside MentionUserRenderer.js then we can just pass accessible to false this will also remove the error

Optional changes

• Disregards this error I have logged another bug report that on opening detail page it throws another warning of Warning: Failed prop type: Invalid prop styleof typeobjectsupplied toPressableWithoutFocus, expected an array.
• If we need to handle it here I have made a Proposal here https://expensify.slack.com/archives/C049HHMV9SM/p1687335105698689?thread_ts=1687326565.653439&cid=C049HHMV9SM

[StevenKKC]

Proposal

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

Console warning on pressing any mentioned user from conversation.

What is the root cause of that problem?

User details is displayed in DetailsPage component, and DetailsPage contains PressableWithoutFocus.

App/src/pages/DetailsPage.js

Lines 150 to 152 in 820fa93

	<PressableWithoutFocus
	style={styles.noOutline}
	onPress={show}

And PressableWithoutFocus has style props with array type.

App/src/components/Pressable/PressableWithoutFocus.js

Line 19 in 820fa93

style: PropTypes.arrayOf(PropTypes.object),

But style prop that is passed to PressableWithoutFocus is not array type, so the console warning is displayed.

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

We can change style prop to array type.

   <PressableWithoutFocus
      style={[styles.noOutline]}
      onPress={show}
   >

What alternative solutions did you explore? (Optional)

None.

[melvin-bot]

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

[bfitzexpensify]

Hey @cristipaval - I can't test on dev, can you confirm this is reproducible? Thanks!

[cristipaval]

I can't reproduce with the latest dev.

[bfitzexpensify]

@dhairyasenjaliya can you still reproduce this?

[dhairyasenjaliya]

I can repro reliably on the latest main @cristipaval @bfitzexpensify

[bfitzexpensify]

Thanks @dhairyasenjaliya - @cristipaval, mind trying again?

[cristipaval]

alright, I could reproduce. Updated the reproducible steps to make them clear.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010aec32706776696c

[melvin-bot]

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

[melvin-bot]

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

[hugolgst]

Proposal

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

Console warnings appear when clicking a user mentioned in the chat:

1. Warning: Failed prop type: Invalid prop 'style' of type 'object' supplied to 'PressableWithoutFocus', expected an array.
2. Warning: Failed prop type: Provide a valid string for accessibilityLabel prop when accessible is true

What is the root cause of that problem?

1. The type of the styling element forwarded to PressableWithoutFocus is wrong.

   App/src/pages/DetailsPage.js

   Line 151 in a23d161

   style={styles.noOutline}

2. The accessibilityLabel property is missing for the button thus making it hard to identify the nature of the button for the visually impaired.

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

1. Wrap the forwarded styling in an array, as it is already done in other elements, e.g.

   App/src/pages/DetailsPage.js

   Line 138 in a23d161

   style={[styles.containerWithSpaceBetween]}

2. Add the accessibilityLabel property with text explaining what the button linked to the user profile picture is doing (enlarging the picture)

What alternative solutions did you explore? (Optional)

I did not explore other solutions.

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

📣 @hugolgst! 📣
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>

[cristipaval]

@mollfpr do you have some time to review the proposal or you want to wait a bit longer for other proposals?

[mollfpr]

Thanks for the ping @cristipaval

I'm reviewing the proposal now.

[mollfpr]

@cristipaval @dhairyasenjaliya I was only able to reproduce the style error.

[dhairyasenjaliya]

Hm on latest pull i can repro style error only

[mollfpr]

@cristipaval I can't reproduce the accessibility error, but we still can fix the style error with @dhairyasenjaliya proposal.

Should we proceed?

[cristipaval]

@cristipaval I can't reproduce the accessibility error, but we still can fix the style error with @dhairyasenjaliya proposal.

Should we proceed?

I guess @Beamanator fixed it already. Yes, let's proceed with the style error.

[melvin-bot]

📣 @mollfpr 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Upwork job

[melvin-bot]

📣 @dhairyasenjaliya You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @dhairyasenjaliya We're missing your Upwork ID to automatically send you an offer for the Reporter role.
Once you apply to the Upwork job, your Upwork ID will be stored and you will be automatically hired for future jobs!

[dhairyasenjaliya]

@mollfpr @cristipaval PR ready for review

[melvin-bot]

@cristipaval, @mollfpr, @bfitzexpensify, @dhairyasenjaliya Whoops! This issue is 2 days overdue. Let's get this updated quick!

[cristipaval]

@bfitzexpensify I don't know why this wasn't automatically updated. The PR was deployed into production 2 days ago. We'll have to do the payments in 5 days.

[bfitzexpensify]

Thanks for catching that @cristipaval

[mollfpr]

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] 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:

I'm unable to track which PR that causing the console error; it's between the migration on PressableWithoutFocus #20202 or the changes on the TouchableWithoutFocus to PressableWithoutFocus in #8483

[@mollfpr] 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:

The console.error can be caught on the PR review. This issue might arise when migrating the generic component and the test is not happening in all the pages, but we have a checklist to check that.

If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] 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.

The error is only shown in DEV, so no regression test is needed. The reviewer checklist already covers this.

[mollfpr]

Melvin did not calculate the date...

@dhairyasenjaliya Got assigned on Jul 5

And the PR merged on Jul 6

So merged PR within 1.042 days 🚀

[bfitzexpensify]

Cool, all payments issued here, BZ checklist complete, so we're all done - thanks for the solid work!