Skip to content
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.

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

fix(tags): update TagProps interface so only the data tag types are exported #5548

Merged
merged 2 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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"
edleeks87 marked this conversation as resolved.
Show resolved Hide resolved
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}
edleeks87 marked this conversation as resolved.
Show resolved Hide resolved
{...filterStyledSystemMarginProps(props)}
>
{validationRedesignOptIn && labelHelp && (
Expand Down