-
Notifications
You must be signed in to change notification settings - Fork 22
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: resolve blip of UI before notices redirect, refactor NProgress l…
…oader (#993)
- Loading branch information
1 parent
0c905a5
commit bc2a7a0
Showing
13 changed files
with
374 additions
and
96 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import { useContext, useEffect } from 'react'; | ||
| import { useFetchers, useNavigation } from 'react-router-dom'; | ||
| import nprogress from 'accessible-nprogress'; | ||
| import { AppContext } from '@edx/frontend-platform/react'; | ||
| import useNotices from './useNotices'; | ||
|
|
||
| // Determines amount of time that must elapse before the | ||
| // NProgress loader is shown in the UI. No need to show it | ||
| // for quick route transitions. | ||
| export const NPROGRESS_DELAY_MS = 300; | ||
|
|
||
| function useNProgressLoader() { | ||
| const { authenticatedUser } = useContext(AppContext); | ||
| const isAuthenticatedUserHydrated = !!authenticatedUser?.profileImage; | ||
| const navigation = useNavigation(); | ||
| const fetchers = useFetchers(); | ||
| const { | ||
| data: noticeRedirectUrl, | ||
| isLoading: isLoadingNotices, | ||
| } = useNotices(); | ||
|
|
||
| const hasNoticeRedirectUrl = !isLoadingNotices && !!noticeRedirectUrl; | ||
| const isAppDataHydrated = isAuthenticatedUserHydrated && !hasNoticeRedirectUrl; | ||
|
|
||
| useEffect(() => { | ||
| const timeoutId = setTimeout(() => { | ||
| const fetchersIdle = fetchers.every((f) => f.state === 'idle'); | ||
| if (navigation.state === 'idle' && fetchersIdle && isAppDataHydrated) { | ||
| nprogress.done(); | ||
| } else { | ||
| nprogress.start(); | ||
| } | ||
| }, NPROGRESS_DELAY_MS); | ||
| return () => clearTimeout(timeoutId); | ||
| }, [navigation, fetchers, isAppDataHydrated]); | ||
|
|
||
| return isAppDataHydrated; | ||
| } | ||
|
|
||
| export default useNProgressLoader; |
139 changes: 139 additions & 0 deletions
139
src/components/app/data/hooks/useNProgressLoader.test.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| import { renderHook } from '@testing-library/react-hooks'; | ||
| import { AppContext } from '@edx/frontend-platform/react'; | ||
| import nprogress from 'accessible-nprogress'; | ||
| import { useFetchers, useNavigation } from 'react-router-dom'; | ||
| import { waitFor } from '@testing-library/react'; | ||
|
|
||
| import useNProgressLoader from './useNProgressLoader'; | ||
| import useNotices from './useNotices'; | ||
|
|
||
| jest.mock('./useNotices'); | ||
|
|
||
| jest.mock('accessible-nprogress', () => ({ | ||
| start: jest.fn(), | ||
| done: jest.fn(), | ||
| })); | ||
|
|
||
| jest.mock('react-router-dom', () => ({ | ||
| ...jest.requireActual('react-router-dom'), | ||
| useNavigation: jest.fn(), | ||
| useFetchers: jest.fn(), | ||
| })); | ||
|
|
||
| const defaultAppContextValue = { | ||
| authenticatedUser: { | ||
| userId: 3, | ||
| }, | ||
| }; | ||
|
|
||
| const appContextValueWithHydratedUser = { | ||
| authenticatedUser: { | ||
| userId: 3, | ||
| profileImage: 'profileImage', | ||
| }, | ||
| }; | ||
|
|
||
| describe('useNProgressLoader', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| useNotices.mockReturnValue({ | ||
| data: null, | ||
| isLoading: false, | ||
| }); | ||
| useNavigation.mockReturnValue({ state: 'idle' }); | ||
| useFetchers.mockReturnValue([]); | ||
| }); | ||
|
|
||
| it('should start nprogress, but not call done with unhydrated authenticated user', async () => { | ||
| const Wrapper = ({ children }) => ( | ||
| <AppContext.Provider value={defaultAppContextValue}> | ||
| {children} | ||
| </AppContext.Provider> | ||
| ); | ||
| const { result } = renderHook(() => useNProgressLoader(), { wrapper: Wrapper }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(nprogress.start).toHaveBeenCalledTimes(1); | ||
| }); | ||
| await waitFor(() => { | ||
| expect(nprogress.done).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| expect(result.current).toBe(false); | ||
| }); | ||
|
|
||
| it('should start nprogress, but not call done with notice redirect url', async () => { | ||
| useNotices.mockReturnValue({ data: 'http://example.com', isLoading: false }); | ||
| const Wrapper = ({ children }) => ( | ||
| <AppContext.Provider value={defaultAppContextValue}> | ||
| {children} | ||
| </AppContext.Provider> | ||
| ); | ||
| const { result } = renderHook(() => useNProgressLoader(), { wrapper: Wrapper }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(nprogress.start).toHaveBeenCalledTimes(1); | ||
| }); | ||
| await waitFor(() => { | ||
| expect(nprogress.done).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| expect(result.current).toBe(false); | ||
| }); | ||
|
|
||
| it('should start nprogress, but not call done with loading navigation state', async () => { | ||
| useNavigation.mockReturnValue({ state: 'loading' }); | ||
| const Wrapper = ({ children }) => ( | ||
| <AppContext.Provider value={appContextValueWithHydratedUser}> | ||
| {children} | ||
| </AppContext.Provider> | ||
| ); | ||
| const { result } = renderHook(() => useNProgressLoader(), { wrapper: Wrapper }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(nprogress.start).toHaveBeenCalledTimes(1); | ||
| }); | ||
| await waitFor(() => { | ||
| expect(nprogress.done).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| expect(result.current).toBe(true); | ||
| }); | ||
|
|
||
| it('should start nprogress, but not call done with non-idle fetchers', async () => { | ||
| useFetchers.mockReturnValue([{ state: 'loading' }]); | ||
| const Wrapper = ({ children }) => ( | ||
| <AppContext.Provider value={appContextValueWithHydratedUser}> | ||
| {children} | ||
| </AppContext.Provider> | ||
| ); | ||
| const { result } = renderHook(() => useNProgressLoader(), { wrapper: Wrapper }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(nprogress.start).toHaveBeenCalledTimes(1); | ||
| }); | ||
| await waitFor(() => { | ||
| expect(nprogress.done).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| expect(result.current).toBe(true); | ||
| }); | ||
|
|
||
| it('should call nprogress done with hydrated user and no notices', async () => { | ||
| const Wrapper = ({ children }) => ( | ||
| <AppContext.Provider value={appContextValueWithHydratedUser}> | ||
| {children} | ||
| </AppContext.Provider> | ||
| ); | ||
| const { result } = renderHook(() => useNProgressLoader(), { wrapper: Wrapper }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(nprogress.done).toHaveBeenCalledTimes(1); | ||
| }); | ||
| await waitFor(() => { | ||
| expect(nprogress.start).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| expect(result.current).toBe(true); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { useEffect } from 'react'; | ||
| import { useQuery } from '@tanstack/react-query'; | ||
|
|
||
| import { queryNotices } from '../../routes/data'; | ||
|
|
||
| /** | ||
| * Responsible for returning the redirect URL for any notice(s) present | ||
| * for the authenticated user. | ||
| */ | ||
| function useNotices() { | ||
| const queryResults = useQuery(queryNotices()); | ||
|
|
||
| useEffect(() => { | ||
| if (!queryResults.data) { | ||
| return; | ||
| } | ||
| window.location.assign(queryResults.data); | ||
| }, [queryResults.data]); | ||
|
|
||
| return queryResults; | ||
| } | ||
|
|
||
| export default useNotices; |
Oops, something went wrong.