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

[$1000] Sigin - Console error when opening a chat #20448

Closed
2 of 6 tasks
kbecciv opened this issue Jun 8, 2023 · 38 comments
Closed
2 of 6 tasks

[$1000] Sigin - Console error when opening a chat #20448

kbecciv opened this issue Jun 8, 2023 · 38 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 Help Wanted Apply this label when an issue is open to proposals by contributors Needs Reproduction Reproducible steps needed

Comments

@kbecciv
Copy link

kbecciv commented Jun 8, 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 any chat

Expected Result:

No console error when opening a chat

Actual Result:

Console error when opening a chat

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.24.4

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

image (57)
image (56)

Expensify/Expensify Issue URL:

Issue reported by: bernhardoj

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686028265538889

View all open jobs on GitHub

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

melvin-bot bot commented Jun 8, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 8, 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

@situchan
Copy link
Contributor

situchan commented Jun 8, 2023

Proposal

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

Console error shows when open chat with anchor

What is the root cause of that problem?

style={{...props.style, ...parentStyle, ...styles.textUnderlinePositionUnder, ...styles.textDecorationSkipInkNone}}

Latest RN doesn't support such keys in style definition and strictly lint-errors them.

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

Define those style definitions in web only.
To do this, move them to styles/utilities with platform specific files, similar to #17908

Also remove this redundant style which was already defined in utilities/cursor:

App/src/styles/styles.js

Lines 2694 to 2696 in 4cad248

cursorPointer: {
cursor: 'pointer',
},

@Pujan92
Copy link
Contributor

Pujan92 commented Jun 8, 2023

I think @spcheema working on it as it occurs after their PR.

@situchan
Copy link
Contributor

situchan commented Jun 8, 2023

Also there's another console error with cursorPointer key. It has the same root cause and should be fixed here.

@kbecciv kbecciv changed the title Android/iOS - Console error when opening a chat Sigin - Console error when opening a chat Jun 8, 2023
@melvin-bot melvin-bot bot added the Overdue label Jun 12, 2023
@tjferriss tjferriss added the External Added to denote the issue can be worked on by a contributor label Jun 13, 2023
@melvin-bot melvin-bot bot changed the title Sigin - Console error when opening a chat [$1000] Sigin - Console error when opening a chat Jun 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0152de705a953964b4

@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jun 13, 2023

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

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

melvin-bot bot commented Jun 13, 2023

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

@tjferriss
Copy link
Contributor

tjferriss commented Jun 13, 2023

I'm not able to reproduce as I don't have a mobile dev environment with access to the console. @sobitneupane or @robertjchen are either of you able to reproduce the bug?

Also, if there is a way for me to reproduce the console error on staging or prod, please let me know :)

@melvin-bot melvin-bot bot removed the Overdue label Jun 13, 2023
@spcheema
Copy link
Contributor

spcheema commented Jun 13, 2023

@tjferriss It's not reproducible on prod. It's console warning which wont block the build. #20462 to fix some other warnings. Lemme know if there is a need to fix this one as well though it's separate bug.

@situchan
Copy link
Contributor

textUnderlinePosition, textDecorationSkipInk console errors are gone but cursor error still remaining. It happens on ReportPreview component.
Reproducible step: request money on any chat

console

@sobitneupane
Copy link
Contributor

@tjferriss

are either of you able to reproduce the bug?

Yes, I can reproduce the issue.

Also, if there is a way for me to reproduce the console error on staging or prod, please let me know :)

I don't think it can be reproduced on staging or prod.

@sobitneupane
Copy link
Contributor

Thanks for the proposal @situchan. Removing cursorPointer solves one warning but following warning still remains.

Screenshot 2023-06-13 at 14 08 37

@spcheema
Copy link
Contributor

Proposed solution

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

Getting console error Failed prop type: Invalid prop stylesof typeobjectsupplied toPressableWithoutFocus, expected an array

What is the root cause of that problem?
Style property of PressableWithoutFocus component expect array but an object is .

Following code in 3 files

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

Files are

  • src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js
  • src/pages/DetailsPage.js
  • src/pages/ProfilePage.js

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

Update code as below

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

What alternative solutions did you explore? (Optional)

NA

@spcheema
Copy link
Contributor

@sobitneupane There was another issue created to fix cursor pointer issue. This solution is only for PressableWithoutFocus component. Happy to consider cursor issue as well if it's still there.

@situchan
Copy link
Contributor

Thanks for the proposal @situchan. Removing cursorPointer solves one warning but following warning still remains.

Screenshot 2023-06-13 at 14 08 37

This is already known and being fixed somewhere - #19545 (comment)

@sobitneupane
Copy link
Contributor

Yup @situchan.PressableWithoutFocus warning is already being fixed in #20202.

The warning in cursor can be fixed by @situchan's proposal.

There was another issue created to fix cursor pointer issue.

@spcheema Can you please link the issue?

@parasharrajat
Copy link
Member

parasharrajat commented Jun 13, 2023

We are already fixing this Pressablewithoutfocus issue and it was known to us

@melvin-bot melvin-bot bot added the Overdue label Jun 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 16, 2023

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

@sobitneupane
Copy link
Contributor

@situchan Is the issue still reproducible?

@melvin-bot melvin-bot bot removed the Overdue label Jun 19, 2023
@situchan
Copy link
Contributor

@situchan Is the issue still reproducible?

yes still

@sobitneupane
Copy link
Contributor

Proposal from @situchan looks good to me.

🎀 👀 🎀 C+ reviewed
cc: @robertjchen

@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

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

@spcheema
Copy link
Contributor

Yup @situchan.PressableWithoutFocus warning is already being fixed in #20202.

The warning in cursor can be fixed by @situchan's proposal.

There was another issue created to fix cursor pointer issue.

@spcheema Can you please link the issue?

@sobitneupane Just for the curiosity sake what we are going to fix in issue?

I think all issues are addressed separately. The original one is fixed in mine PR.

@situchan
Copy link
Contributor

@spcheema can you please link that PR? This is still reproducible on main

@spcheema
Copy link
Contributor

spcheema commented Jun 19, 2023

@situchan I have fixed textDecoration related warnings which are gone. And cursor pointer is fixed here #20448 (comment) incorrect link. I saw there was another issue /PR related to cursor pointer but I can't find it anymore.

@spcheema
Copy link
Contributor

@situchan #20589

@parasharrajat
Copy link
Member

@sobitneupane Can you please clarify what are we trying to solve here?

@sobitneupane
Copy link
Contributor

Following console warning on opening chat.

@parasharrajat
Copy link
Member

Ok, So we fixed it already. Can you please recheck if you are able to reproduce this on main?

@sobitneupane
Copy link
Contributor

@parasharrajat Can you please link the issue and PR which fixed it?

@parasharrajat
Copy link
Member

Here is the PR #20968

@sobitneupane
Copy link
Contributor

Oops Sorry @situchan. The solution is already implemented. Thanks for the proposal though.

@spcheema
Copy link
Contributor

spcheema commented Jun 21, 2023

@situchan @sobitneupane @parasharrajat I should have located before. I saw #20589 as dupe but I couldn't find it. Thought this issue is quite old.

@melvin-bot
Copy link

melvin-bot bot commented Jun 22, 2023

@robertjchen @tjferriss @sobitneupane this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@sobitneupane
Copy link
Contributor

@tjferriss We should be good to close this issue.

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 Help Wanted Apply this label when an issue is open to proposals by contributors Needs Reproduction Reproducible steps needed
Projects
None yet
Development

No branches or pull requests

8 participants