Skip to content

[Chakra v3 upgrade] @W-18672580 migrate offline components #2684

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.

Sign up for GitHub

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

Merged
sf-jie-dai merged 7 commits into from
Jul 1, 2025
Merged

[Chakra v3 upgrade] @W-18672580 migrate offline components #2684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b656e9d
update: migrate offline components from v2 to v3
sf-jie-dai Jun 27, 2025
921af09
add unit test coverage report & cleanup
sf-jie-dai Jun 30, 2025
3bb0a95
class name clean up
sf-jie-dai Jun 30, 2025
3eee4df
use renderWithRouter to test offline-boundary
sf-jie-dai Jun 30, 2025
3bf6378
use newer chakra UI component instead of basic html tags
sf-jie-dai Jun 30, 2025
45cfe62
wrap user.click with act
sf-jie-dai Jun 30, 2025
73e901e
clean up comments
sf-jie-dai Jun 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/extension-chakra-storefront/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ module.exports = {
createTestGlob('pages/home'),
createTestGlob('pages/cart'),
createTestGlob('pages/product-list'),
createTestGlob('components/offline-banner'),
createTestGlob('components/offline-boundary'),
// createTestGlob('pages/login'), // TODO: enable after Account page has been migrated
createTestGlob('pages/login-redirect'),
createTestGlob('pages/social-login-redirect'),
Expand Down Expand Up @@ -95,6 +97,7 @@ module.exports = {
'non-pwa/**/*.{js,jsx}',
'worker/**/*.{js,jsx}',
'scripts/generator/*.{js,jsx}',
'src/**/*.{js,jsx}',
Copy link
Contributor Author

@sf-jie-dai sf-jie-dai Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add unit test coverage report for extension-chakra-storefront folder

'!app/pages/test-container/**/*.{js,jsx}',
'!app/utils/test-utils.js',
'!app/mocks/*.js',
Expand Down
10 changes: 5 additions & 5 deletions packages/extension-chakra-storefront/src/components/offline-banner/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import React from 'react'
import {useIntl} from 'react-intl'

// Components
import {Alert, AlertDescription, Flex} from '@chakra-ui/react'
Copy link
Contributor Author

@sf-jie-dai sf-jie-dai Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In chakra v3, everything imported from Alert alone, AlertDescription is not needed anymore

import {Alert, Flex} from '@chakra-ui/react'

// Icons
import {AlertIcon} from '../../components/icons'
Expand All @@ -20,17 +20,17 @@ import {AlertIcon} from '../../components/icons'
const OfflineBanner = ({...props}) => {
const intl = useIntl()
return (
<Alert status="warning" {...props}>
<Alert.Root status="warning" colorPalette="blue" {...props}>
<Flex align="center">
<AlertIcon mr={2} />
<AlertDescription>
<Alert.Title>
{intl.formatMessage({
id: 'offline_banner.description.browsing_offline_mode',
defaultMessage: "You're currently browsing in offline mode"
})}
</AlertDescription>
</Alert.Title>
</Flex>
</Alert>
</Alert.Root>
)
}

Expand Down
27 changes: 7 additions & 20 deletions packages/extension-chakra-storefront/src/components/offline-boundary/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,9 @@
import React from 'react'
import PropTypes from 'prop-types'
import {withRouter} from 'react-router-dom'
import {Button} from '@chakra-ui/react'
import {Button, Box, Heading, Text} from '@chakra-ui/react'
import {AlertIcon} from '../../components/icons'

// import Button from '@salesforce/pwa-kit-react-sdk/components/button'
// import Icon from '@salesforce/pwa-kit-react-sdk/components/icon'

/**
* OfflineBoundary is a React Error boundary that catches errors thrown when
* dynamically loading pages and renders a fallback.
Expand Down Expand Up @@ -68,24 +65,14 @@ class OfflineBoundary extends React.Component {
return (
<React.Fragment>
Copy link
Contributor Author

@sf-jie-dai sf-jie-dai Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every component in offline-boundary is v3 compatible already.
Removed legacy classnames

{chunkLoadError ? (
<div className="c-offline-boundary u-direction-column u-text-align-center u-padding-top u-padding-bottom">
<Box>
<AlertIcon />

<h1 className="u-margin-bottom-md u-text-family">
You are currently offline
</h1>

<p className="u-margin-bottom-lg">
<Heading>You are currently offline</Heading>
<Text>
{"We couldn't load the next page on this connection. Please try again."}
</p>

<Button
className="u-width-block-full pw--primary qa-retry-button"
onClick={() => this.clearError()}
>
Retry Connection
</Button>
</div>
</Text>
<Button onClick={() => this.clearError()}>Retry Connection</Button>
</Box>
) : (
children
)}
Expand Down
182 changes: 90 additions & 92 deletions packages/extension-chakra-storefront/src/components/offline-boundary/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,27 @@
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import React from 'react'
import {screen} from '@testing-library/react'
// import userEvent from '@testing-library/user-event'

import OfflineBoundary from '../../components/offline-boundary/index'
import {screen, act} from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import {ChakraProvider} from '@chakra-ui/react'
import theme from '../../theme'
import {renderWithRouter} from '../../utils/test-utils'

// class ChunkLoadError extends Error {
// constructor(...params) {
// // Pass remaining arguments (including vendor specific ones) to parent constructor
// super(...params)
// this.name = 'ChunkLoadError'
// }
// }
import OfflineBoundary, {UnwrappedOfflineBoundary} from '../../components/offline-boundary/index'

// Custom render function that combines Router and Chakra contexts
const renderWithRouterAndChakra = (component) => {
const ComponentWithChakra = () => <ChakraProvider value={theme}>{component}</ChakraProvider>
return renderWithRouter(<ComponentWithChakra />)
}

class ChunkLoadError extends Error {
constructor(...params) {
// Pass remaining arguments (including vendor specific ones) to parent constructor
super(...params)
this.name = 'ChunkLoadError'
}
}

describe('The OfflineBoundary', () => {
beforeEach(() => {
Expand All @@ -31,7 +39,7 @@ describe('The OfflineBoundary', () => {
})

test('should render its children', () => {
renderWithRouter(
renderWithRouterAndChakra(
Copy link
Contributor

@alexvuong alexvuong Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why cant we use renderWithProviders?

Copy link
Contributor Author

@sf-jie-dai sf-jie-dai Jun 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In offline-boundary's index.jsx I saw

export default withRouter(OfflineBoundary)

From my understanding, with withRouter, the error boundary becomes "route-aware", in which normal renderWithProviders wouldn't be able to cover this use case. So use renderWithRouter for this special component

<OfflineBoundary isOnline={true}>
<div id="child">child</div>
</OfflineBoundary>
Expand All @@ -40,84 +48,74 @@ describe('The OfflineBoundary', () => {
expect(screen.getByText(/child/i)).toBeInTheDocument()
})

// TODO: Fix flaky/broken test
// eslint-disable-next-line jest/no-commented-out-tests
// test('should render the error splash when a child throws a chunk load error', () => {
// const ThrowingComponent = () => {
// throw new ChunkLoadError()
// }
// renderWithRouter(
// <OfflineBoundary isOnline={true}>
// <div>
// <ThrowingComponent />
// <div id="child">child</div>
// </div>
// </OfflineBoundary>
// )

// expect(screen.getByRole('img', {name: /offline cloud/i})).toBeInTheDocument()
// expect(
// screen.getByRole('heading', {name: /you are currently offline/i})
// ).toBeInTheDocument()
// expect(screen.queryByText(/child/i)).not.toBeInTheDocument()
// })

// TODO: Fix flaky/broken test
// eslint-disable-next-line jest/no-commented-out-tests
// test('should re-throw errors that are not chunk load errors', () => {
// const ThrowingComponent = () => {
// throw new Error('Anything else')
// }
// expect(() => {
// renderWithRouter(
// <OfflineBoundary isOnline={true}>
// <div>
// <ThrowingComponent />
// <div id="child">child</div>
// </div>
// </OfflineBoundary>
// )
// }).toThrow()
// })

// TODO: Fix flaky/broken test
// eslint-disable-next-line jest/no-commented-out-tests
// test('should attempt to reload the page when the user clicks retry', () => {
// let firstRender = true
// const ThrowingOnceComponent = () => {
// if (firstRender) {
// firstRender = false
// throw new ChunkLoadError()
// } else {
// return <div id="child">child</div>
// }
// }
// renderWithRouter(
// <OfflineBoundary isOnline={true}>
// <ThrowingOnceComponent />
// </OfflineBoundary>
// )

// expect(screen.getByRole('img', {name: /offline cloud/i})).toBeInTheDocument()
// expect(
// screen.getByRole('heading', {name: /you are currently offline/i})
// ).toBeInTheDocument()
// expect(screen.queryByText(/child/i)).not.toBeInTheDocument()

// userEvent.click(screen.getByRole('button', {name: /retry connection/i}))
// expect(screen.getByText(/child/i)).toBeInTheDocument()
// expect(screen.queryByRole('img', {name: /offline cloud/i})).not.toBeInTheDocument()
// expect(
// screen.queryByRole('heading', {name: /you are currently offline/i})
// ).not.toBeInTheDocument()
// })

// TODO: Fix flaky/broken test
// eslint-disable-next-line jest/no-commented-out-tests
// test('should derive state from a chunk load error', () => {
// const derived = UnwrappedOfflineBoundary.getDerivedStateFromError(
// new ChunkLoadError('test')
// )
// expect(derived).toEqual({chunkLoadError: true})
// })
test('should render the error splash when a child throws a chunk load error', () => {
const ThrowingComponent = () => {
throw new ChunkLoadError()
}
renderWithRouterAndChakra(
<OfflineBoundary isOnline={true}>
<div>
<ThrowingComponent />
<div id="child">child</div>
</div>
</OfflineBoundary>
)

expect(screen.getByRole('img', {hidden: true})).toBeInTheDocument()
expect(
screen.getByRole('heading', {name: /you are currently offline/i})
).toBeInTheDocument()
expect(screen.queryByText(/child/i)).not.toBeInTheDocument()
})

test('should re-throw errors that are not chunk load errors', () => {
const ThrowingComponent = () => {
throw new Error('Anything else')
}
expect(() => {
renderWithRouterAndChakra(
<OfflineBoundary isOnline={true}>
<div>
<ThrowingComponent />
<div id="child">child</div>
</div>
</OfflineBoundary>
)
}).toThrow()
})

test('should call clearError when retry button is clicked', async () => {
const user = userEvent.setup()
const ThrowingComponent = () => {
throw new ChunkLoadError()
}
const clearErrorSpy = jest.spyOn(UnwrappedOfflineBoundary.prototype, 'clearError')

renderWithRouterAndChakra(
<OfflineBoundary isOnline={true}>
<ThrowingComponent />
</OfflineBoundary>
)

expect(screen.getByRole('img', {hidden: true})).toBeInTheDocument()
expect(
screen.getByRole('heading', {name: /you are currently offline/i})
).toBeInTheDocument()

const retryButton = screen.getByRole('button', {name: /retry connection/i})
await act(async () => {
await user.click(retryButton)
})

expect(clearErrorSpy).toHaveBeenCalled()

clearErrorSpy.mockRestore()
})

test('should derive state from a chunk load error', () => {
const derived = UnwrappedOfflineBoundary.getDerivedStateFromError(
new ChunkLoadError('test')
)
expect(derived).toEqual({chunkLoadError: true})
})
})
14 changes: 6 additions & 8 deletions packages/extension-chakra-storefront/src/components/with-layout/with-layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ import CheckoutHeader from '../../pages/checkout/partials/checkout-header'
import CheckoutFooter from '../../pages/checkout/partials/checkout-footer'
import Footer from '../footer'
import Header from '../header'
// import OfflineBanner from '../offline-banner'
// import OfflineBoundary from '../offline-boundary'
import OfflineBanner from '../offline-banner'
import OfflineBoundary from '../offline-boundary'
import Seo from '../seo'
import ScrollToTop from '../scroll-to-top'

Expand Down Expand Up @@ -274,7 +274,6 @@ const withLayout = <P extends object>(WrappedComponent: React.ComponentType<P>)
</Seo>

<ScrollToTop />

<Box id="app" display="flex" flexDirection="column" flex={1}>
{/*TODO: recreating this component because @chakra-ui/skip-nav does not have V3 version*/}
{/*<SkipNavLink zIndex="skipLink">Skip to Content</SkipNavLink>*/}
Expand Down Expand Up @@ -314,7 +313,7 @@ const withLayout = <P extends object>(WrappedComponent: React.ComponentType<P>)
<CheckoutHeader />
)}
</Box>
{/*{!isOnline && <OfflineBanner />}*/}
{!isOnline && <OfflineBanner />}
<AddToCartModalProvider>
{/*TODO: recreating this component because @chakra-ui/skip-nav does not have V3 version*/}
{/*<SkipNavContent*/}
Expand All @@ -333,10 +332,9 @@ const withLayout = <P extends object>(WrappedComponent: React.ComponentType<P>)
flexDirection="column"
flex="1"
>
{/*<OfflineBoundary isOnline={false}>*/}

<WrappedComponent {...(props as P)} />
{/*</OfflineBoundary>*/}
<OfflineBoundary isOnline={isOnline}>
<WrappedComponent {...(props as P)} />
</OfflineBoundary>
</Box>
{/*</SkipNavContent>*/}

Expand Down
Loading