feat: add LoadingProvider to the app #1105

acezard · 2024-01-05T14:22:41Z

`useLoadingOverlay` Hook Documentation

Overview

The useLoadingOverlay hook provides a simple and flexible way to manage a loading overlay in the application. It allows you to show or hide an overlay with a customizable message, ideal for indicating loading states during asynchronous operations like API calls or data processing.

Using the hook

Import and use the useLoadingOverlay hook in your components to control the overlay.

import { useLoadingOverlay } from 'app/view/Loading/LoadingOverlay';

const MyComponent = () => {
    const { showOverlay, hideOverlay } = useLoadingOverlay();

    // Use showOverlay and hideOverlay as needed
};

API

`showOverlay(message?: string): void`

Description: Shows the loading overlay with an optional message.
Parameters:
- message: (Optional) The message to display on the overlay.
Example:
```
showOverlay('Loading data...');
```

`hideOverlay(): void`

Description: Hides the loading overlay.
Example:
```
hideOverlay();
```

Best Practices

Explicit Control: Always ensure to hide the overlay explicitly once the loading process is complete.
Error Handling: In case of asynchronous operations, use hideOverlay in both .then() and .catch() to handle successful and error scenarios.
Cleanup: For operations tied to the component lifecycle, use the useEffect cleanup function to hide the overlay.

Example Usage

const FileDownloader = () => {
    const { showOverlay, hideOverlay } = useLoadingOverlay();

    const downloadFiles = async () => {
        showOverlay('Downloading files...');
        try {
            await downloadFileLogic();
        } catch (error) {
            console.error(error);
            // Handle error
        } finally {
            hideOverlay();
        }
    };

    return (
        <button onClick={downloadFiles}>Download</button>
    );
};

Screenshot

zatteo · 2024-01-05T16:20:37Z

Like the documentation and PR description a lot.

It seems like we have now two approach to display something "above everything" :

overlay like here with a context
OAuthClientLimitxxx with events (made by @Ldoppea)

My opinion is that for the moment it is good to try both to see advantages/drawbacks.

Ldoppea · 2024-01-05T16:49:40Z

It seems like we have now two approach to display something "above everything" :

overlay like here with a context

OAuthClientLimitxxx with events (made by @Ldoppea)

Both approaches have their pros and their cons.

@acezard 's implementation seems great since the Overlay is displayed directly from the Provider component. This was not possible for OAuthClientLimitxxx because it had to be displayed in the HomeScreen component to allow CozyAppScreen to be displayed on top of him.

Events based way has the advantage of easily preventing react's rerenders and can be called from everywhere (even from out-of-react methods), but the main drawback is that we don't rely on React's mechanisms anymore and we are drifting towards an object/event based application instead of a functional one.

My questions about the Overlay implement are the following:

Where will the Provider be placed? Around <App />? Or somewhere else? Do we expect to declare it in multiple places?
- I suppose the only component that can be on top of him is the LockScreen so we should check that
Will we need to display the Overlay from a post-me localMethod?
Will calling showOverlay() trigger a rerender on the children component or on the caller component?

acezard · 2024-01-07T14:02:55Z

@Ldoppea

Where will the Provider be placed? Around ? Or somewhere else? Do we expect to declare it in multiple places?

I decided to place it directly under the <ErrorProvider /> inside the <Nav /> wrapper. It is quite low in the "provider pyramid" but still higher than everything that is an actual view, except the LockScreen as you mentioned it. I'm not sure how to handle that we should discuss about it. But you're right that it displays above it if put above in the React tree.

Will we need to display the Overlay from a post-me localMethod?

Yes it is needed. That's why the provider should be above the <NativeIntentProvider>, so some component can use both providers without issues, or even the NativeIntentProvider itself.

Here is the implementation

export const useShareFiles = (): (() => Promise<void>) => {
  const { showOverlay, hideOverlay } = useLoadingOverlay()
  const { handleError } = useError()
  const { t } = useI18n()
  const client = useClient()

  return async (arg = ''): Promise<void> => {
    const fileIds = JSON.parse(arg) as string[]

    try {
      showOverlay()

      if (!client) throw new Error('Client is undefined')

      await fetchFilesByIds(client, fileIds)
    } catch (error) {
      if (getErrorMessage(error) === 'User did not share') return

      handleError(
        t('errors.shareFiles', {
          postProcess: 'interval',
          count: fileIds.length
        })
      )

      OsReceiveLogger.error('useShareFiles: error', error)
    } finally {
      hideOverlay()
    }
  }
}

const InnerNav = ({ client, setClient }) => {
  const shareFiles = useShareFiles()

  return (
    <NativeIntentProvider
      localMethods={localMethods(client, {
        shareFiles
      })}
    >
  )
}

Will calling showOverlay() trigger a rerender on the children component or on the caller component?

It calls a render cycle only on component consuming the context values returned by the hook. Other children components are not expected to get a render cycle, in theory or in some tests I did (if HomeView use the hook, HomeScreen isn't triggering a render when we hide the overlay)

Extracting logic

Ldoppea

We still have to move the LockScreenWrapper (and its corresponding SafeAreaProvider)on top of the LoadingOverlay but everything else is good 👍

acezard requested review from Ldoppea and zatteo as code owners January 5, 2024 14:22

acezard force-pushed the feat--add-LoadingProvider-to-the-app branch from e130254 to f018ee3 Compare January 5, 2024 14:57

zatteo approved these changes Jan 5, 2024

View reviewed changes

feat: add LoadingProvider to the app 8000

97f2c18

acezard force-pushed the feat--add-LoadingProvider-to-the-app branch from f018ee3 to 025cc4c Compare January 7, 2024 13:49

refactor: decouple shareFiles from osReceive state

b3eb221

acezard force-pushed the feat--add-LoadingProvider-to-the-app branch from 025cc4c to b3eb221 Compare January 7, 2024 14:40

refactor: make the shareFiles hook lighter

c0e3875

Extracting logic

Ldoppea approved these changes Jan 12, 2024

View reviewed changes

fix: handle zindex between loading/lock screens

84e5c45

acezard merged commit d3bdeb8 into master Jan 19, 2024

acezard deleted the feat--add-LoadingProvider-to-the-app branch January 19, 2024 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add LoadingProvider to the app #1105

feat: add LoadingProvider to the app #1105

feat: add LoadingProvider to the app #1105

feat: add LoadingProvider to the app #1105

Conversation

useLoadingOverlay Hook Documentation

Overview

Using the hook

API

showOverlay(message?: string): void

hideOverlay(): void

Best Practices

Example Usage

Screenshot

Choose a reason for hiding this comment

`useLoadingOverlay` Hook Documentation

`showOverlay(message?: string): void`

`hideOverlay(): void`