Skip to content

Commit

Permalink
bug/Checkbox declaration file inconsistency (#133)
Browse files Browse the repository at this point in the history
* Add "strictNullChecks", fix two cases where typing needs updating in line with checks.

* Revert to .defaultProps

* Throw error on default HeadingLevel.tsx case

* Warn on invalid prop, test to cover

* Better dev warning for heading level

---------

Co-authored-by: Thomas Judd-Cooper <[email protected]>
  • Loading branch information
KaiSpencer and Tomdango authored Apr 12, 2023
1 parent 4993b4e commit 14ae393
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ exports[`DoDontList matches snapshot: DoDontList-Do 1`] = `
>
<HeadingLevel
className="nhsuk-do-dont-list__label"
headingLevel="h3"
>
Do
</HeadingLevel>
Expand All @@ -33,7 +32,6 @@ exports[`DoDontList matches snapshot: DoDontList-Dont 1`] = `
>
<HeadingLevel
className="nhsuk-do-dont-list__label"
headingLevel="h3"
>
Don't
</HeadingLevel>
Expand Down
15 changes: 7 additions & 8 deletions src/components/table/TableHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ export const isTableCell = (child: ReactNode): child is ReactElement => {
};

export const getHeadingsFromChildren = (children: ReactNode): string[] => {
return React.Children
.map(children, child => {
if (isTableCell(child)) {
return child.props.children.toString();
}
return null;
})
.filter(Boolean);
const headings: string[] = [];
React.Children.map(children, (child) => {
if (isTableCell(child)) {
headings.push(child.props.children.toString());
}
});
return headings;
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ exports[`WarningCallout matches snapshot 1`] = `
>
<HeadingLevel
className="nhsuk-warning-callout__label"
headingLevel="h3"
>
<h3
className="nhsuk-warning-callout__label"
Expand Down
8 changes: 4 additions & 4 deletions src/util/HeadingLevel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export type HeadingLevelType =
| 'H5'
| 'H6';

const HeadingLevel: React.FC<HeadingLevelProps> = ({ headingLevel, ...rest }) => {
const HeadingLevel: React.FC<HeadingLevelProps> = ({ headingLevel='h3', ...rest }) => {
switch (headingLevel.toLowerCase()) {
case 'h1':
return <h1 {...rest} />;
Expand All @@ -32,10 +32,10 @@ const HeadingLevel: React.FC<HeadingLevelProps> = ({ headingLevel, ...rest }) =>
return <h5 {...rest} />;
case 'h6':
return <h6 {...rest} />;
default:
console.error(`HeadingLevel: Invalid headingLevel prop: ${headingLevel}`);
return <h3 {...rest} />;
}
};
HeadingLevel.defaultProps = {
headingLevel: 'h3',
};

export default HeadingLevel;
11 changes: 11 additions & 0 deletions src/util/__tests__/HeadingLevel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,15 @@ describe('HeadingLevel', () => {
h6Element.unmount();
H6Element.unmount();
});

it("console.warn when headingLevel is invalid", () => {
// eslint-disable-next-line @typescript-eslint/no-empty-function
const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
// @ts-expect-error - testing invalid prop
shallow(<HeadingLevel headingLevel="h7" />);
expect(consoleSpy).toHaveBeenCalledTimes(1);
expect(consoleSpy).toHaveBeenCalledWith(
'HeadingLevel: Invalid headingLevel prop: h7');
consoleSpy.mockRestore();
});
});
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"types": ["react", "jest", "node"],
"allowSyntheticDefaultImports": true,
"esModuleInterop": true,
"skipLibCheck": true
"skipLibCheck": true,
"strictNullChecks": true
},
"include": ["src/**/*"],
"exclude": [
Expand Down

0 comments on commit 14ae393

Please sign in to comment.