From f8eca7e3782ca8b8b37a7d959f1281e385057dd0 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 26 Aug 2022 15:14:10 -0400 Subject: [PATCH 1/3] fix(Modal, Dropdown, TreeView): updates to resolve strict TS errors --- .../src/components/Dropdown/DropdownItem.tsx | 2 +- .../Dropdown/DropdownToggleCheckbox.tsx | 2 +- .../Dropdown/InternalDropdownItem.tsx | 20 +++---- .../src/components/Dropdown/Toggle.tsx | 16 +++--- .../react-core/src/components/Modal/Modal.tsx | 4 +- .../src/components/Modal/ModalBoxTitle.tsx | 2 +- .../src/components/TreeView/TreeView.tsx | 2 +- .../components/TreeView/TreeViewListItem.tsx | 2 +- .../src/components/TreeView/TreeViewRoot.tsx | 54 ++++++++++--------- 9 files changed, 54 insertions(+), 50 deletions(-) diff --git a/packages/react-core/src/components/Dropdown/DropdownItem.tsx b/packages/react-core/src/components/Dropdown/DropdownItem.tsx index 69cd54302af..4166e335a96 100644 --- a/packages/react-core/src/components/Dropdown/DropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/DropdownItem.tsx @@ -40,7 +40,7 @@ export interface DropdownItemProps extends InternalDropdownItemProps, OUIAProps /** Custom item rendering that receives the DropdownContext */ customChild?: React.ReactNode; /** tabIndex to use, null to unset it */ - tabIndex?: number | null; + tabIndex?: number; /** An image to display within the DropdownItem, appearing before any component children */ icon?: React.ReactNode; /** Initial focus on the item when the menu is opened (Note: Only applicable to one of the items) */ diff --git a/packages/react-core/src/components/Dropdown/DropdownToggleCheckbox.tsx b/packages/react-core/src/components/Dropdown/DropdownToggleCheckbox.tsx index e014eee7da4..ac95a99b4a9 100644 --- a/packages/react-core/src/components/Dropdown/DropdownToggleCheckbox.tsx +++ b/packages/react-core/src/components/Dropdown/DropdownToggleCheckbox.tsx @@ -44,7 +44,7 @@ export class DropdownToggleCheckbox extends React.Component) => { - this.props.onChange((event.target as HTMLInputElement).checked, event); + this.props.onChange?.((event.target as HTMLInputElement).checked, event); }; calculateChecked = () => { diff --git a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx index 32d3561a54b..4c70aa49d4f 100644 --- a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx @@ -87,7 +87,7 @@ export class InternalDropdownItem extends React.Component { if (!isDisabled && !isAriaDisabled) { - onClick(event); - onSelect(event); + onClick?.(event); + onSelect?.(event); } }} id={id} diff --git a/packages/react-core/src/components/Dropdown/Toggle.tsx b/packages/react-core/src/components/Dropdown/Toggle.tsx index c03bbd5b7e1..3b1d9175537 100644 --- a/packages/react-core/src/components/Dropdown/Toggle.tsx +++ b/packages/react-core/src/components/Dropdown/Toggle.tsx @@ -89,7 +89,7 @@ export class Toggle extends React.Component { const clickedOnToggle = parentRef && parentRef.current && parentRef.current.contains(event.target as Node); const clickedWithinMenu = menuRef && menuRef.contains && menuRef.contains(event.target as Node); if (isOpen && !(clickedOnToggle || clickedWithinMenu)) { - onToggle(false, event); + onToggle?.(false, event); } }; @@ -103,8 +103,8 @@ export class Toggle extends React.Component { (event.key === KeyTypes.Escape || event.key === 'Tab') && (escFromToggle || escFromWithinMenu) ) { - this.props.onToggle(false, event); - this.buttonRef.current.focus(); + this.props.onToggle?.(false, event); + this.buttonRef.current?.focus(); } }; @@ -118,15 +118,15 @@ export class Toggle extends React.Component { } event.preventDefault(); - this.props.onToggle(!this.props.isOpen, event); + this.props.onToggle?.(!this.props.isOpen, event); } else if ((event.key === 'Enter' || event.key === ' ') && !this.props.isOpen) { if (!this.props.bubbleEvent) { event.stopPropagation(); } event.preventDefault(); - this.props.onToggle(!this.props.isOpen, event); - this.props.onEnter(); + this.props.onToggle?.(!this.props.isOpen, event); + this.props.onEnter?.(); } }; @@ -167,11 +167,11 @@ export class Toggle extends React.Component { isPlain && styles.modifiers.plain, isText && styles.modifiers.text, isPrimary && styles.modifiers.primary, - buttonVariantStyles[toggleVariant], + toggleVariant && buttonVariantStyles[toggleVariant], className )} type={type || 'button'} - onClick={event => onToggle(!isOpen, event)} + onClick={event => onToggle?.(!isOpen, event)} aria-expanded={isOpen} aria-haspopup={ariaHasPopup} onKeyDown={event => this.onKeyDown(event)} diff --git a/packages/react-core/src/components/Modal/Modal.tsx b/packages/react-core/src/components/Modal/Modal.tsx index 0d80d0ff46a..10d4cb95f6e 100644 --- a/packages/react-core/src/components/Modal/Modal.tsx +++ b/packages/react-core/src/components/Modal/Modal.tsx @@ -26,7 +26,7 @@ export interface ModalProps extends React.HTMLProps, OUIAProps { /** Optional title label text for screen readers */ titleLabel?: string; /** Id to use for Modal Box label */ - 'aria-labelledby'?: string | null; + 'aria-labelledby'?: string; /** Accessible descriptor of modal */ 'aria-label'?: string; /** Id to use for Modal Box descriptor */ @@ -143,7 +143,7 @@ export class Modal extends React.Component { } }; - isEmpty = (value: string | null) => value === null || value === undefined || value === ''; + isEmpty = (value: string | null | undefined) => value === null || value === undefined || value === ''; componentDidMount() { const { diff --git a/packages/react-core/src/components/Modal/ModalBoxTitle.tsx b/packages/react-core/src/components/Modal/ModalBoxTitle.tsx index fb8cd14be74..dc821d291b0 100644 --- a/packages/react-core/src/components/Modal/ModalBoxTitle.tsx +++ b/packages/react-core/src/components/Modal/ModalBoxTitle.tsx @@ -38,7 +38,7 @@ export const ModalBoxTitle: React.FunctionComponent = ({ ...props }: ModalBoxTitleProps) => { const [hasTooltip, setHasTooltip] = React.useState(false); - const h1 = React.useRef(); + const h1 = React.useRef(null); const label = titleLabel || (isVariantIcon(titleIconVariant) ? `${capitalize(titleIconVariant)} alert:` : titleLabel); const variantIcons = { success: , diff --git a/packages/react-core/src/components/TreeView/TreeView.tsx b/packages/react-core/src/components/TreeView/TreeView.tsx index 5c75daab3c4..598a932ac0c 100644 --- a/packages/react-core/src/components/TreeView/TreeView.tsx +++ b/packages/react-core/src/components/TreeView/TreeView.tsx @@ -101,7 +101,7 @@ export const TreeView: React.FunctionComponent = ({ {data.map(item => ( > { - checked?: boolean | null; + checked?: boolean; } export interface TreeViewListItemProps { diff --git a/packages/react-core/src/components/TreeView/TreeViewRoot.tsx b/packages/react-core/src/components/TreeView/TreeViewRoot.tsx index dcb729095db..6303e744ea7 100644 --- a/packages/react-core/src/components/TreeView/TreeViewRoot.tsx +++ b/packages/react-core/src/components/TreeView/TreeViewRoot.tsx @@ -26,16 +26,16 @@ export class TreeViewRoot extends React.Component { window.addEventListener('keydown', this.props.hasChecks ? this.handleKeysCheckbox : this.handleKeys); } if (this.props.hasChecks) { - const firstToggle = this.treeRef.current.getElementsByClassName('pf-c-tree-view__node-toggle')[0] as HTMLElement; + const firstToggle = this.treeRef.current?.getElementsByClassName('pf-c-tree-view__node-toggle')[0] as HTMLElement; if (firstToggle) { firstToggle.tabIndex = 0; } - const firstInput = this.treeRef.current.getElementsByTagName('INPUT')[0] as HTMLElement; + const firstInput = this.treeRef.current?.getElementsByTagName('INPUT')[0] as HTMLElement; if (firstInput) { firstInput.tabIndex = 0; } } else { - (this.treeRef.current.getElementsByClassName('pf-c-tree-view__node')[0] as HTMLElement).tabIndex = 0; + (this.treeRef.current?.getElementsByClassName('pf-c-tree-view__node')[0] as HTMLElement).tabIndex = 0; } } @@ -51,7 +51,7 @@ export class TreeViewRoot extends React.Component { } const activeElement = document.activeElement; const key = event.key; - const treeItems = Array.from(this.treeRef.current.getElementsByClassName('pf-c-tree-view__node')).filter( + const treeItems = Array.from(this.treeRef.current?.getElementsByClassName('pf-c-tree-view__node')).filter( el => !el.classList.contains('pf-m-disabled') ); @@ -72,20 +72,22 @@ export class TreeViewRoot extends React.Component { ); if (['ArrowLeft', 'ArrowRight'].includes(key)) { - const isExpandable = activeElement.firstElementChild.firstElementChild.classList.contains( + const isExpandable = activeElement?.firstElementChild?.firstElementChild?.classList.contains( 'pf-c-tree-view__node-toggle' ); - const isExpanded = activeElement.closest('li').classList.contains('pf-m-expanded'); + const isExpanded = activeElement?.closest('li')?.classList.contains('pf-m-expanded'); if (key === 'ArrowLeft') { if (isExpandable && isExpanded) { (activeElement as HTMLElement).click(); } else { - const parentList = activeElement.closest('ul').parentElement; - if (parentList.tagName !== 'DIV') { - const parentButton = parentList.querySelector('button'); + const parentList = activeElement?.closest('ul')?.parentElement; + if (parentList?.tagName !== 'DIV') { + const parentButton = parentList?.querySelector('button'); (activeElement as HTMLElement).tabIndex = -1; - parentButton.tabIndex = 0; - parentButton.focus(); + if (parentButton) { + parentButton.tabIndex = 0; + parentButton.focus(); + } } } } else { @@ -93,11 +95,13 @@ export class TreeViewRoot extends React.Component { (activeElement as HTMLElement).tabIndex = -1; (activeElement as HTMLElement).click(); const childElement = activeElement - .closest('li') - .querySelector('ul > li') - .querySelector('button'); - childElement.tabIndex = 0; - childElement.focus(); + ?.closest('li') + ?.querySelector('ul > li') + ?.querySelector('button'); + if (childElement) { + childElement.tabIndex = 0; + childElement.focus(); + } } } event.preventDefault(); @@ -116,7 +120,7 @@ export class TreeViewRoot extends React.Component { event.preventDefault(); } - const treeNodes = Array.from(this.treeRef.current.getElementsByClassName('pf-c-tree-view__node')); + const treeNodes = Array.from(this.treeRef.current?.getElementsByClassName('pf-c-tree-view__node')); handleArrows( event, @@ -131,21 +135,21 @@ export class TreeViewRoot extends React.Component { if (['ArrowLeft', 'ArrowRight'].includes(key)) { if (key === 'ArrowLeft') { - if (activeElement.tagName === 'INPUT') { - activeElement.parentElement.previousSibling && + if (activeElement?.tagName === 'INPUT') { + activeElement?.parentElement?.previousSibling && (activeElement.parentElement.previousSibling as HTMLElement).focus(); - } else if (activeElement.previousSibling) { - if (activeElement.previousElementSibling.tagName === 'SPAN') { + } else if (activeElement?.previousSibling) { + if (activeElement.previousElementSibling?.tagName === 'SPAN') { (activeElement.previousSibling.firstChild as HTMLElement).focus(); } else { (activeElement.previousSibling as HTMLElement).focus(); } } } else { - if (activeElement.tagName === 'INPUT') { - activeElement.parentElement.nextSibling && (activeElement.parentElement.nextSibling as HTMLElement).focus(); - } else if (activeElement.nextSibling) { - if (activeElement.nextElementSibling.tagName === 'SPAN') { + if (activeElement?.tagName === 'INPUT') { + activeElement.parentElement?.nextSibling && (activeElement.parentElement.nextSibling as HTMLElement).focus(); + } else if (activeElement?.nextSibling) { + if (activeElement.nextElementSibling?.tagName === 'SPAN') { (activeElement.nextSibling.firstChild as HTMLElement).focus(); } else { (activeElement.nextSibling as HTMLElement).focus(); From 8477cbde6b09b08edf1faf36bed5c04a9fbca49f Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 9 Sep 2022 11:58:45 -0400 Subject: [PATCH 2/3] Resolve conflicts --- packages/react-core/src/components/Modal/Modal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-core/src/components/Modal/Modal.tsx b/packages/react-core/src/components/Modal/Modal.tsx index 10d4cb95f6e..5c5e8553143 100644 --- a/packages/react-core/src/components/Modal/Modal.tsx +++ b/packages/react-core/src/components/Modal/Modal.tsx @@ -121,7 +121,7 @@ export class Modal extends React.Component { handleEscKeyClick = (event: KeyboardEvent): void => { const { onEscapePress } = this.props; if (event.key === KeyTypes.Escape && this.props.isOpen) { - onEscapePress ? onEscapePress(event) : this.props.onClose(); + onEscapePress ? onEscapePress(event) : this.props.onClose?.(); } }; From 6db6b2f8066ba1d8adab01c6b2a45277633c4830 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 9 Sep 2022 14:46:20 -0400 Subject: [PATCH 3/3] Update interfaces to omit props --- packages/react-core/src/components/Dropdown/DropdownItem.tsx | 4 ++-- .../react-core/src/components/TreeView/TreeViewListItem.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-core/src/components/Dropdown/DropdownItem.tsx b/packages/react-core/src/components/Dropdown/DropdownItem.tsx index 4166e335a96..7658199665b 100644 --- a/packages/react-core/src/components/Dropdown/DropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/DropdownItem.tsx @@ -3,7 +3,7 @@ import { InternalDropdownItemProps, InternalDropdownItem } from './InternalDropd import { DropdownArrowContext } from './dropdownConstants'; import { useOUIAProps, OUIAProps } from '../../helpers'; -export interface DropdownItemProps extends InternalDropdownItemProps, OUIAProps { +export interface DropdownItemProps extends Omit, OUIAProps { /** Anything which can be rendered as dropdown item */ children?: React.ReactNode; /** Classes applied to root element of dropdown item */ @@ -40,7 +40,7 @@ export interface DropdownItemProps extends InternalDropdownItemProps, OUIAProps /** Custom item rendering that receives the DropdownContext */ customChild?: React.ReactNode; /** tabIndex to use, null to unset it */ - tabIndex?: number; + tabIndex?: number | null; /** An image to display within the DropdownItem, appearing before any component children */ icon?: React.ReactNode; /** Initial focus on the item when the menu is opened (Note: Only applicable to one of the items) */ diff --git a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx index c9b7acf2630..c4bee30aa15 100644 --- a/packages/react-core/src/components/TreeView/TreeViewListItem.tsx +++ b/packages/react-core/src/components/TreeView/TreeViewListItem.tsx @@ -6,8 +6,8 @@ import { TreeViewDataItem } from './TreeView'; import { Badge } from '../Badge'; import { GenerateId } from '../../helpers/GenerateId/GenerateId'; -export interface TreeViewCheckProps extends Partial> { - checked?: boolean; +export interface TreeViewCheckProps extends Omit>, 'checked'> { + checked?: boolean | null; } export interface TreeViewListItemProps {