Skip to content

Commit

Permalink
fix(tags): update TagProps interface so only the data tag types are e…
Browse files Browse the repository at this point in the history
…xported

Updates the `TagProps` interface to ensure only the `data-` props are exported and extended in the
public interfaces. A new private `RestProps` interface has been added to support passing the rest
props object into the `tags` util. The previous implementation was preventing `intellisense` from
displaying the correct types for the `Sidebar` component; instead it was being assigned `any`.

fix #5534
  • Loading branch information
edleeks87 committed Oct 19, 2022
1 parent 67afb0a commit e6d14ba
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 39 deletions.
16 changes: 8 additions & 8 deletions src/__internal__/utils/helpers/tags/tags.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
interface DataProps {
/** Identifier used for testing purposes, applied to the root element of the component. */
"data-element"?: string;
/** Identifier used for testing purposes, applied to the root element of the component. */
"data-role"?: string;
interface RestProps {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[restKeys: string]: any;
}

export interface TagProps extends DataProps {
/** Identifier used for testing purposes, applied to the root element of the component. */
export interface TagProps {
/** @private @ignore Identifier used for testing purposes, applied to the root element of the component. */
"data-component"?: string;
/** Identifier used for testing purposes, applied to the root element of the component. */
"data-element"?: string;
/** Identifier used for testing purposes, applied to the root element of the component. */
"data-role"?: string;
}

/**
* Builds props object containing top level data tags
*/
function tagComponent(
componentName: string | undefined,
props: DataProps
props: TagProps & RestProps
): TagProps {
const tagProps: TagProps = {
"data-component": componentName,
Expand Down
2 changes: 1 addition & 1 deletion src/components/dialog-full-screen/dialog-full-screen.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export interface DialogFullScreenProps extends ModalProps {
/** The ARIA role to be applied to the DialogFullscreen container */
role?: string;
/** an optional array of refs to containers whose content should also be reachable by tabbing from the dialog */
focusableContainers?: React.MutableRefObject<HTMLElement>[];
focusableContainers?: React.MutableRefObject<HTMLElement | null>[];
}

declare function DialogFullScreen(props: DialogFullScreenProps): JSX.Element;
Expand Down
8 changes: 7 additions & 1 deletion src/components/dialog/dialog-test.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,20 @@ export default {
},
};

interface StoryProps {
stickyFooter: boolean;
titleSpecialCharacters: string;
subtitleSpecialCharacters: string;
}

export const Default = ({
stickyFooter,
title,
titleSpecialCharacters,
subtitle,
subtitleSpecialCharacters,
...args
}: Partial<DialogProps>) => {
}: Partial<DialogProps> & StoryProps) => {
const [date, setDate] = useState("01/06/2020");
const [isOpen, setIsOpen] = useState(true);
const handleCancel = (
Expand Down
6 changes: 5 additions & 1 deletion src/components/dialog/dialog.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import useIsStickyFooterForm from "../../hooks/__internal__/useIsStickyFooterFor

const PADDING_VALUES = [0, 1, 2, 3, 4, 5, 6, 7, 8] as const;
type PaddingValues = typeof PADDING_VALUES[number];
// TODO FE-5408 will investigate why React.RefObject<T> produces a failed prop type when current = null
type CustomRefObject<T> = {
current?: T | null;
};

export interface ContentPaddingInterface {
p?: PaddingValues;
Expand Down Expand Up @@ -82,7 +86,7 @@ export interface DialogProps extends ModalProps, TagProps {
/** Padding to be set on the Dialog content */
contentPadding?: ContentPaddingInterface;
/** an optional array of refs to containers whose content should also be reachable by tabbing from the dialog */
focusableContainers?: React.MutableRefObject<HTMLElement | null>[];
focusableContainers?: CustomRefObject<HTMLElement>[];
}

export const Dialog = ({
Expand Down
7 changes: 1 addition & 6 deletions src/components/dialog/dialog.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ describe("Dialog", () => {
mount(
<Dialog
onCancel={() => {}}
onConfirm={() => {}}
open
subtitle="Test"
title="Test"
Expand Down Expand Up @@ -372,7 +371,6 @@ describe("Dialog", () => {
size="small"
className="foo"
onCancel={onCancel}
onConfirm={() => {}}
height="500"
role="dialog"
data-element="bar"
Expand Down Expand Up @@ -423,7 +421,6 @@ describe("Dialog", () => {
wrapper = mount(
<Dialog
onCancel={() => {}}
onConfirm={() => {}}
open
subtitle="Test"
title="Test"
Expand All @@ -435,9 +432,7 @@ describe("Dialog", () => {
describe("when title or subtitle are not set", () => {
it(`does not render aria-labelledby pointing at the title element or
an aria-describedby attribute pointing at the subtitle element`, () => {
wrapper = mount(
<Dialog onCancel={() => {}} onConfirm={() => {}} open />
);
wrapper = mount(<Dialog onCancel={() => {}} open />);

expect(
wrapper.find('[aria-describedby="carbon-dialog-subtitle"]').length
Expand Down
1 change: 0 additions & 1 deletion src/components/dialog/dialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ export const DynamicContent: StoryFn = () => {
size="medium"
open={isOpen}
title="Dialog with dynamic content"
isLoading="isLoading"
onCancel={() => setIsOpen(false)}
>
{isLoading ? (
Expand Down
19 changes: 12 additions & 7 deletions src/components/sidebar/sidebar.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import { filterStyledSystemPaddingProps } from "../../style/utils";
import useIsStickyFooterForm from "../../hooks/__internal__/useIsStickyFooterForm";
import { TagProps } from "../../__internal__/utils/helpers/tags/tags";

// TODO FE-5408 will investigate why React.RefObject<T> produces a failed prop type when current = null
type CustomRefObject<T> = {
current?: T | null;
};

export interface SidebarContextProps {
isInSidebar?: boolean;
}
Expand All @@ -25,13 +30,13 @@ export interface SidebarProps extends PaddingProps, TagProps {
"aria-describedby"?: string;
/**
* Prop to specify the aria-label of the component.
* To be used only when the title prop is not defined, and the component is not labelled by any internal element.
* To be used only when the header prop is not defined, and the component is not labelled by any internal element.
*/
"aria-label"?: string;
/**
* Prop to specify the aria-labeledby property of the component
* To be used when the title prop is a custom React Node,
* or the component is labelled by an internal element other than the title.
* To be used when the header prop is a custom React Node,
* or the component is labelled by an internal element other than the header.
*/
"aria-labelledby"?: string;
/** Modal content */
Expand Down Expand Up @@ -62,7 +67,7 @@ export interface SidebarProps extends PaddingProps, TagProps {
| "large"
| "extra-large";
/** an optional array of refs to containers whose content should also be reachable by tabbing from the sidebar */
focusableContainers?: React.MutableRefObject<HTMLElement>[];
focusableContainers?: CustomRefObject<HTMLElement>[];
}

export const Sidebar = React.forwardRef<HTMLDivElement, SidebarProps>(
Expand All @@ -86,7 +91,7 @@ export const Sidebar = React.forwardRef<HTMLDivElement, SidebarProps>(
ref
) => {
const locale = useLocale();
const { current: titleId } = useRef<string>(createGuid());
const { current: headerId } = useRef<string>(createGuid());
const hasStickyFooter = useIsStickyFooterForm(children);

const sidebarRef = useRef<HTMLDivElement | null>(null);
Expand Down Expand Up @@ -126,7 +131,7 @@ export const Sidebar = React.forwardRef<HTMLDivElement, SidebarProps>(
aria-describedby={ariaDescribedBy}
aria-label={ariaLabel}
aria-labelledby={
!ariaLabelledBy && !ariaLabel ? titleId : ariaLabelledBy
!ariaLabelledBy && !ariaLabel ? headerId : ariaLabelledBy
}
ref={setRefs}
position={position}
Expand All @@ -135,7 +140,7 @@ export const Sidebar = React.forwardRef<HTMLDivElement, SidebarProps>(
onCancel={onCancel}
role={role}
>
{header && <SidebarHeader id={titleId}>{header}</SidebarHeader>}
{header && <SidebarHeader id={headerId}>{header}</SidebarHeader>}
{closeIcon()}
<Box
data-element="sidebar-content"
Expand Down
9 changes: 1 addition & 8 deletions src/components/sidebar/sidebar.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,7 @@ describe("Sidebar", () => {
beforeEach(() => {
spy = jest.fn();
wrapper = mount(
<Sidebar
open
title="Test"
className="custom-class"
data-role="baz"
data-element="bar"
onCancel={spy}
>
<Sidebar open data-role="baz" data-element="bar" onCancel={spy}>
<Textbox />
<Textbox />
<Textbox />
Expand Down
7 changes: 3 additions & 4 deletions src/components/sidebar/sidebar.stories.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -281,16 +281,15 @@ This may occasionally be useful with things like Toasts where they persist on th
const [isDialogOpen, setIsDialogOpen] = useState(false);
const [isToast1Open, setIsToast1Open] = useState(false);
const [isToast2Open, setIsToast2Open] = useState(false);
const toast1Ref = useRef();
const toast2Ref = useRef();
const toast1Ref = useRef(null);
const toast2Ref = useRef(null);
return (
<>
<Button onClick={() => setIsDialogOpen(true)}>Open sidebar</Button>
<Sidebar
open={isDialogOpen}
onCancel={() => setIsDialogOpen(false)}
title="Title"
subtitle="Subtitle"
header={<Typography variant="h3">Sidebar header</Typography>}
focusableContainers={[toast1Ref, toast2Ref]}
>
<Form
Expand Down
2 changes: 0 additions & 2 deletions src/components/textbox/textbox.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,6 @@ export const Textbox = ({
validationRedesignOptIn ? undefined : validationIconId
}
validationRedesignOptIn={validationRedesignOptIn}
size={size}
readOnly={readOnly}
{...filterStyledSystemMarginProps(props)}
>
{validationRedesignOptIn && labelHelp && (
Expand Down

0 comments on commit e6d14ba

Please sign in to comment.