lexical icon indicating copy to clipboard operation
lexical copied to clipboard

Load img error UI

Open zurfyx opened this issue 1 year ago • 9 comments

Currently, if an image fails to load because of a network error, page is down etc. the Node is silently inserted but no UI is shown. Ideally, we show some piece of UI that shows that the image failed to load somehow and that it's also easy to remove.

I.e. image

zurfyx avatar Jan 29 '24 06:01 zurfyx

Hello, I am enthusiastic about tackling this issue. Given my experience in handling UI issues, I would appreciate it if you could assign this task to me.

SaxenaShiv avatar Jan 29 '24 09:01 SaxenaShiv

Hello again,

After thorough investigation, I identified the cause behind the UI going blank when the user faces network issues or page loading problems. The main issue stems from the lazy loading of a crucial component, leading to the error message: Uncaught TypeError: Failed to fetch dynamically imported module: http://localhost:3000/src/nodes/ImageComponent.tsx. This error occurs because the component wasn't loaded initially, and when the network connection is lost, it triggers this error, resulting in a blank UI.

To tackle this issue, I made adjustments to the code by replacing the dynamic import statement with a synchronous import for the ImageComponent. This alteration ensures that the component is loaded at the beginning, resolving the error that arises when the network is disconnected. Consequently, even in offline scenarios, the UI remains functional, and the uploaded image is displayed successfully.

P.S.: After implementing these changes, the bundle size increased slightly from 8.8 MB to 8.9 MB.

SaxenaShiv avatar Jan 30 '24 07:01 SaxenaShiv

I've identified a potential solution to the problem, but unfortunately, the Suspense fallback doesn't seem to be functioning as expected. Despite implementing the suspense fallback in both the ImageComponent and ImageNode components within the node directory, the issue persists, and I'm having difficulty identifying their parent component.

Upon further investigation, it appears that the parent component should also implement the suspense library fallback. However, I'm struggling to locate the parent component that is utilizing the suspense library, or perhaps this is beyond my current understanding. If you notice and comprehend the issue, I would appreciate your assistance in resolving it.

SaxenaShiv avatar Feb 05 '24 11:02 SaxenaShiv

I think the problem being described here is with the implementation of ImageComponent and in particular the way LazyImage works. useSuspenseImage has no provision for when an image fails to load in a reasonable amount of time, it never resolves the promise. At the same time, it has a Suspense with null fallback, so it will not render until that promise resolves (which may never happen). The lowest effort workaround would be to race the promise with a timeout, which should let the component render and the browser can show a broken image.

function useSuspenseImage(src: string) {
  if (!imageCache.has(src)) {
    throw new Promise((resolve) => {
      const img = new Image();
      img.src = src;
      img.onload = () => {
        imageCache.add(src);
        resolve(null);
      };
      // Resolve in 5 seconds if the image fails to load promptly
      setTimeout(resolve, 5000, null);
    });
  }
}

etrepum avatar Feb 10 '24 01:02 etrepum

Thank you for the suggestion. I tried the modification you provided, but unfortunately, the error persists. I've also checked for console error messages, but haven't found a resolution yet. Any further guidance or insights would be greatly appreciated.

SaxenaShiv avatar Feb 10 '24 03:02 SaxenaShiv

I've been thinking about an alternative approach. What if we add a condition to the lazy load function so that when an error occurs, we load a placeholder image like 'error-img.png'? This way, we can visually indicate the error without hanging the promise indefinitely. What are your thoughts on this approach?

SaxenaShiv avatar Feb 10 '24 03:02 SaxenaShiv

My mistake, the imageSet cache needs to be populated even if it fails otherwise it will simply retry

etrepum avatar Feb 10 '24 04:02 etrepum

Steps to reproduce:

https://github.com/facebook/lexical/assets/1125900/bd9d192f-74b0-4f39-b8cd-6defef17b357

Expected result:

Display some piece of UI that shows that the image failed to load. Ex, following image:

image

StyleT avatar Apr 05 '24 21:04 StyleT

Okay I understood this UI should appear when the image uploading request is unable to fullfill due to networking issues.

SaxenaShiv avatar Apr 06 '24 14:04 SaxenaShiv