mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: UX - Error logs in development from Empty state component (#466)
Fixes #452 When the empty state is rendered with a non-textual element (which it turns out all our current empty states are, because they have a `<button />` component as a call to action), this noisy error log was showing up in the `console`: ``` Warning: validateDOMNesting(...): <div> cannot appear as a descendant of <p>. at div at div at p at Typography (webpack-internal:///./node_modules/@material-ui/core/esm/Typography/Typography.js:166:28) at WithStyles (webpack-internal:///./node_modules/@material-ui/styles/esm/withStyles/withStyles.js:64:31) at div at StyledComponent (webpack-internal:///./node_modules/@material-ui/styles/esm/styled/styled.js:95:28) at EmptyState (webpack-internal:///./src/components/EmptyState/index.tsx:47:25) ... at ProjectsPage (webpack-internal:///./src/pages/projects/index.tsx:37:18) at Routes (webpack-internal:///./node_modules/react-router/index.js:275:5) at ThemeProvider (webpack-internal:///./node_modules/@material-ui/styles/esm/ThemeProvider/ThemeProvider.js:44:24) at UserProvider (webpack-internal:///./src/contexts/UserContext.tsx:100:55) at SWRConfig$1 (webpack-internal:///./node_modules/swr/dist/index.esm.js:501:23) at Router (webpack-internal:///./node_modules/react-router/index.js:209:15) at BrowserRouter (webpack-internal:///./node_modules/react-router-dom/index.js:118:5) at App ``` The issue was that the `description` prop could either be a `string` or an actual `React` component, but was always rendered as a child of a `<Typography />` component. The `<Typography>` component internally renders as a `<p>`, which is not valid to nest `<div>`s inside. The fix is to not nest inside a `<Typography />` block, but an actual `<div />`.
This commit is contained in:
@@ -11,6 +11,7 @@ module.exports = {
|
||||
preset: "ts-jest",
|
||||
|
||||
roots: ["<rootDir>"],
|
||||
setupFilesAfterEnv: ["./jest.setup.ts"],
|
||||
transform: {
|
||||
"^.+\\.tsx?$": "ts-jest",
|
||||
},
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
// Helper utility to fail jest tests if a console.error is logged
|
||||
// Pulled from this blog post:
|
||||
// https://www.benmvp.com/blog/catch-warnings-jest-tests/
|
||||
|
||||
// For now, I limited this to just 'error' - but failing on warnings
|
||||
// would be a nice next step! We may need to filter out some noise
|
||||
// from material-ui though.
|
||||
const CONSOLE_FAIL_TYPES = ["error" /* 'warn' */]
|
||||
|
||||
// Throw errors when a `console.error` or `console.warn` happens
|
||||
// by overriding the functions
|
||||
CONSOLE_FAIL_TYPES.forEach((logType: string) => {
|
||||
// Suppressing the no-explicit-any to override certain console functions for testing
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
const consoleAsAny = global.console as any
|
||||
consoleAsAny[logType] = (message: string): void => {
|
||||
throw new Error(`Failing due to console.${logType} while running test!\n\n${message}`)
|
||||
}
|
||||
})
|
||||
|
||||
// This is needed because we are compiling under `--isolatedModules`
|
||||
export {}
|
||||
@@ -11,4 +11,25 @@ describe("EmptyState", () => {
|
||||
// Then
|
||||
await screen.findByText("Hello, world")
|
||||
})
|
||||
|
||||
it("renders description text", async () => {
|
||||
// When
|
||||
render(<EmptyState message="Hello, world" description="Friendly greeting" />)
|
||||
|
||||
// Then
|
||||
await screen.findByText("Hello, world")
|
||||
await screen.findByText("Friendly greeting")
|
||||
})
|
||||
|
||||
it("renders description component", async () => {
|
||||
// Given
|
||||
const description = <button title="Click me" />
|
||||
|
||||
// When
|
||||
render(<EmptyState message="Hello, world" description={description} />)
|
||||
|
||||
// Then
|
||||
await screen.findByText("Hello, world")
|
||||
await screen.findByRole("button")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -6,7 +6,7 @@ import Typography from "@material-ui/core/Typography"
|
||||
|
||||
export interface EmptyStateProps {
|
||||
/** Text Message to display, placed inside Typography component */
|
||||
message: React.ReactNode
|
||||
message: string
|
||||
/** Longer optional description to display below the message */
|
||||
description?: React.ReactNode
|
||||
button?: ButtonProps
|
||||
@@ -23,8 +23,6 @@ export interface EmptyStateProps {
|
||||
export const EmptyState: React.FC<EmptyStateProps> = (props) => {
|
||||
const { message, description, button, ...boxProps } = props
|
||||
const styles = useStyles()
|
||||
|
||||
const descClassName = `${styles.description}`
|
||||
const buttonClassName = `${styles.button} ${button && button.className ? button.className : ""}`
|
||||
|
||||
return (
|
||||
@@ -32,11 +30,7 @@ export const EmptyState: React.FC<EmptyStateProps> = (props) => {
|
||||
<Typography variant="h5" color="textSecondary" className={styles.header}>
|
||||
{message}
|
||||
</Typography>
|
||||
{description && (
|
||||
<Typography variant="body2" color="textSecondary" className={descClassName}>
|
||||
{description}
|
||||
</Typography>
|
||||
)}
|
||||
{description && <div className={styles.description}>{description}</div>}
|
||||
{button && <Button variant="contained" color="primary" {...button} className={buttonClassName} />}
|
||||
</Box>
|
||||
)
|
||||
@@ -59,6 +53,8 @@ const useStyles = makeStyles(
|
||||
description: {
|
||||
marginTop: theme.spacing(2),
|
||||
marginBottom: theme.spacing(1),
|
||||
color: theme.palette.text.secondary,
|
||||
fontSize: theme.typography.body2.fontSize,
|
||||
},
|
||||
button: {
|
||||
marginTop: theme.spacing(2),
|
||||
|
||||
Reference in New Issue
Block a user