search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.35k stars 2.78k forks source link

[$1000] Web - Chat - Offline avatar is not updated until page refresh #18057

Open kbecciv opened 1 year ago

kbecciv commented 1 year ago

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. Go to https://staging.new.expensify.com/
  2. Login to an account which has more than 20 chats
  3. Disable the internet connection
  4. Navigate to search page
  5. Scroll and select a chat with offline default avatar
  6. Enable the internet connection

Expected Result:

Avatar should be updated to default online image

Actual Result:

Offline avatar gets grey and is not updated until page refresh.

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.6.0

Reproducible in staging?: Yes

Reproducible in production?: yes

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

https://user-images.githubusercontent.com/93399543/234732792-7e5240dd-2119-40a7-aae9-42fae89cfc9d.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0152f84f02db9bee1c
  • Upwork Job ID: 1673533068774096896
  • Last Price Increase: 2023-07-04
iwiznia commented 1 year ago

Also, I assume you tested other browsers and the behavior is different there? If so, which ones did you test?

s77rt commented 1 year ago

@iwiznia

did you find a chrome bug report for this?

No

How do you know it is a bug that will be fixed vs expected behavior that will never change?

We don't know that

What are the alternatives?

Prevent image from loading when offline, but this would also mean to not load images that are cached. The drawback is cached images will not work once we go offline.

I assume you tested other browsers and the behavior is different there? If so, which ones did you test?

Tested on Firefox and WebKit

iwiznia commented 1 year ago

Prevent image from loading when offline, but this would also mean to not load images that are cached. The drawback is cached images will not work once we go offline.

Yeah, that's not great. Any others?

Also, we sure that's chrome's behavior and that's not some quirk of our code? Can you create a small test file showing the broken behavior of chrome please?

iwiznia commented 1 year ago

I did a small test with this file and can't reproduce the behavior you mention:

<html>
  <body>
    <img id='a' src="https://www.algosolutions.com/wp-content/uploads/2021/05/algo.png">
<button onClick="document.getElementById('a').remove(); document.body.innerHTML = '<img id=\'a\' src=\'https://www.algosolutions.com/wp-content/uploads/2021/05/algo.png\'>'">load</button>

Steps:

s77rt commented 1 year ago

The bug only exists for css urls, use background-image style instead of img tag. I guess the alternative here is to use some RN image library that does not relay on background-image style or make RNW does not relay on it.

iwiznia commented 1 year ago

Ohh I see, can you provide a simple file like I did that shows this behavior please? I am not sure I get exactly what you mean

s77rt commented 1 year ago

Screenshot from 2023-07-12 20-13-13

RNW seems to use the img tag just for accessibility(?) The img seems to have opacity set to 0.

https://github.com/necolas/react-native-web/blob/b10c66351eae55a8ea798a1217254d7a2658c5b1/packages/react-native-web/src/exports/Image/index.js#L409

s77rt commented 1 year ago

I will work on a reproduction example that is outside E/App.

s77rt commented 1 year ago

@iwiznia Here https://codesandbox.io/s/polished-microservice-f99pf8

https://github.com/Expensify/App/assets/16493223/7253f11d-e51e-4a05-a900-4b74a86832c3

melvin-bot[bot] commented 1 year ago

@iwiznia, @s77rt, @sophiepintoraetz, @crazy-coding Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

s77rt commented 1 year ago

@iwiznia Any thoughts here? Should we look for why RNW uses background-image in the first place? It's pretty weird

iwiznia commented 1 year ago

Oh sorry, we had a discussion in slack (prviate channel though https://expensify.slack.com/archives/C03TQ48KC/p1689198663603339) and forgot to update here.

I think we are not going to add hacks to work around a browser bug and we probably should instead report this as a bug to chrome and see if they accept it or not.

s77rt commented 1 year ago

Reported https://bugs.chromium.org/p/chromium/issues/detail?id=1466924

iwiznia commented 1 year ago

Awesome thanks!

I doubt this will have much activity for a while since we need to wait what chrome says, so not unassigning myself, but if this needs an engineer in the future, @sophiepintoraetz please unassign and re-assign another engineer since I am going on sabbatical

sophiepintoraetz commented 1 year ago

I'll drop this one to weekly in the meantime - @s77rt let me know how this bug report tracks!

s77rt commented 1 year ago

I think the issue has been accepted. Still untriaged though

sophiepintoraetz commented 1 year ago

Still waiting on Chrome!

melvin-bot[bot] commented 1 year ago

This issue has not been updated in over 15 days. @iwiznia, @s77rt, @sophiepintoraetz, @crazy-coding eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

s77rt commented 1 year ago

Still waiting on Chrome. Keeping this as Monthly seems good.

thesahindia commented 1 year ago

Hey @s77rt, can you please take a loot at https://github.com/Expensify/App/issues/25447 and confirm if it's a dupe?

dukenv0307 commented 1 year ago

@s77rt My proposal here will also fix this issue and it doesn't have the drawback mentioned here. If we implement that we don't have to wait for Chrome.

Could you take a look? Thank you.

cc @thesahindia

s77rt commented 1 year ago

@thesahindia Seems like a dupe.

@dukenv0307 The reported bug on Chrome has been accepted and it's in open state for now. The best solution is to do nothing. This is not.a bug from our end.

thesahindia commented 1 year ago

Thanks for the confirmation @s77rt.

sophiepintoraetz commented 11 months ago

Thinking on this, do we close the issue? Seeing as it's still with Chrome (are they guaranteed to fix it?), and we can't do anything about it? @s77rt @iwiznia

iwiznia commented 11 months ago

We can do that or keep it open, does not bother much 🤷

s77rt commented 11 months ago

I think they have a commit to fix the bug https://chromium.googlesource.com/chromium/src/+/c04f52aa42ef09dd9cc5e50b8b3b1809165a6a3b Not sure if this released to the latest version or not yet (seeing the issue still marked as Open I guess not)

melvin-bot[bot] commented 10 months ago

@iwiznia, @s77rt, @sophiepintoraetz, @crazy-coding, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

izarutskaya commented 5 months ago

Issue is reproducible in build v1.4.69-0 Offline avatar gets grey and is not updated until page refresh.

https://github.com/Expensify/App/assets/115492554/4ad09cdb-55af-4368-ba68-73c8445cf073

s77rt commented 2 months ago

@izarutskaya Is this still reproducible? Can you try update the browser first