From 225b8694715951edf60c93daa87f6b922ce4943e Mon Sep 17 00:00:00 2001 From: redallen Date: Mon, 30 Mar 2020 14:13:26 -0400 Subject: [PATCH 1/6] fix(app-launcher): less aggressive router linking --- .../src/components/Dropdown/DropdownItem.tsx | 6 ++-- .../Dropdown/InternalDropdownItem.tsx | 32 +++++++++++-------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/react-core/src/components/Dropdown/DropdownItem.tsx b/packages/react-core/src/components/Dropdown/DropdownItem.tsx index 3f155041ffb..27e60371110 100644 --- a/packages/react-core/src/components/Dropdown/DropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/DropdownItem.tsx @@ -53,8 +53,6 @@ export const DropdownItem: React.FunctionComponent = ({ context={context} role="menuitem" tabIndex={-1} - // eslint-disable-next-line react/no-children-prop - children={children} className={className} component={component} variant={variant} @@ -68,7 +66,9 @@ export const DropdownItem: React.FunctionComponent = ({ additionalChild={additionalChild} customChild={customChild} {...props} - /> + > + {children} + )} ); diff --git a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx index 0043f642098..c0868e74c56 100644 --- a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx @@ -13,7 +13,7 @@ export interface InternalDropdownItemProps extends React.HTMLProps {({ onSelect, itemClass, disabledClass, hoverClass }) => { if (this.props.role === 'separator') { - classes = className; + classes = css( + variant === 'icon' && styles.modifiers.icon, + className + ); } else { - classes = css(isDisabled && disabledClass, isHovered && hoverClass, className); + classes = css( + variant === 'icon' && styles.modifiers.icon, + className, + isDisabled && disabledClass, + isHovered && hoverClass, + itemClass + ); } if (customChild) { return React.cloneElement(customChild as React.ReactElement, { @@ -199,23 +207,19 @@ export class InternalDropdownItem extends React.Component {renderWithTooltip( - isChildReactElement ? ( - React.cloneElement(children as React.ReactElement, { + React.isValidElement(component) ? ( + React.cloneElement(component as React.ReactElement, { ...additionalProps, - ref: this.ref, + href, id, - className: css(classes, itemClass, variant === 'icon' && styles.modifiers.icon) + className: classes }) ) : ( {children} From 7a7871be6a46d5c67216738e242caf2833dfa804 Mon Sep 17 00:00:00 2001 From: redallen Date: Mon, 30 Mar 2020 14:46:51 -0400 Subject: [PATCH 2/6] support component={} api --- .../examples/ApplicationLauncher.md | 15 ++++++++------- .../components/Dropdown/InternalDropdownItem.tsx | 9 +++------ .../src/components/Dropdown/examples/Dropdown.md | 10 ++++++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/react-core/src/components/ApplicationLauncher/examples/ApplicationLauncher.md b/packages/react-core/src/components/ApplicationLauncher/examples/ApplicationLauncher.md index 7e0d860589e..6b5e6606e2d 100644 --- a/packages/react-core/src/components/ApplicationLauncher/examples/ApplicationLauncher.md +++ b/packages/react-core/src/components/ApplicationLauncher/examples/ApplicationLauncher.md @@ -11,6 +11,7 @@ To add a tooltip, use the `tooltip` prop and optionally add more tooltip props b import { ApplicationLauncher, ApplicationLauncherContent, ApplicationLauncherIcon, ApplicationLauncherText, ApplicationLauncherItem, ApplicationLauncherGroup, ApplicationLauncherSeparator, Text } from '@patternfly/react-core'; import { HelpIcon, StarIcon } from '@patternfly/react-icons'; +import { Link } from '@reach/router'; import pfIcon from './pf-logo-small.svg'; ## Examples @@ -59,6 +60,7 @@ class SimpleApplicationLauncher extends React.Component { ```js title=Router-link import React from 'react'; +import { Link } from '@reach/router'; import { ApplicationLauncher, ApplicationLauncherItem, ApplicationLauncherContent, Text } from '@patternfly/react-core'; class SimpleApplicationLauncher extends React.Component { @@ -86,14 +88,13 @@ class SimpleApplicationLauncher extends React.Component { color: 'var(--pf-c-app-launcher__menu-item--Color)', textDecoration: 'none' }; - // Using Text component below to demonstrate, but in reality you'd use a router Link component const appLauncherItems = [ - Router link - + + @reach/router Link + } />, - Router link with icon - + + @reach/router Link with icon + } />, diff --git a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx index c0868e74c56..33b61adf84e 100644 --- a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx @@ -58,7 +58,6 @@ export class InternalDropdownItem extends React.Component | React.KeyboardEvent | MouseEvent) => undefined as any, @@ -67,8 +66,6 @@ export class InternalDropdownItem extends React.Component {}, sendRef: () => {} }, - id: undefined, - componentID: undefined, enterTriggersArrowDown: false }; @@ -209,10 +206,10 @@ export class InternalDropdownItem extends React.Component, { - ...additionalProps, href, - id, - className: classes + id: componentID, + className: classes, + ...additionalProps }) ) : ( - Link - + + @reach/router Link + + } /> ]; return ( From f1cba6d43480e816163f3364d70a2475e35a5f8d Mon Sep 17 00:00:00 2001 From: redallen Date: Mon, 30 Mar 2020 15:01:48 -0400 Subject: [PATCH 3/6] remove default empty href --- .../src/components/Dropdown/DropdownItem.tsx | 2 +- .../components/Dropdown/InternalDropdownItem.tsx | 13 ++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/react-core/src/components/Dropdown/DropdownItem.tsx b/packages/react-core/src/components/Dropdown/DropdownItem.tsx index 27e60371110..1bcb41878fe 100644 --- a/packages/react-core/src/components/Dropdown/DropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/DropdownItem.tsx @@ -36,7 +36,7 @@ export const DropdownItem: React.FunctionComponent = ({ variant = 'item', isDisabled = false, isHovered = false, - href = '', + href, tooltip = null, tooltipProps = {}, listItemClassName, diff --git a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx index 33b61adf84e..1860127f351 100644 --- a/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx +++ b/packages/react-core/src/components/Dropdown/InternalDropdownItem.tsx @@ -171,10 +171,7 @@ export class InternalDropdownItem extends React.Component {({ onSelect, itemClass, disabledClass, hoverClass }) => { if (this.props.role === 'separator') { - classes = css( - variant === 'icon' && styles.modifiers.icon, - className - ); + classes = css(variant === 'icon' && styles.modifiers.icon, className); } else { classes = css( variant === 'icon' && styles.modifiers.icon, @@ -212,13 +209,7 @@ export class InternalDropdownItem extends React.Component + {children} ) From 0ace8d6c5b9df51211d32fc14fcc6fdd8c716dbc Mon Sep 17 00:00:00 2001 From: redallen Date: Mon, 30 Mar 2020 15:01:53 -0400 Subject: [PATCH 4/6] update snapshots --- .../ApplicationLauncher.test.tsx.snap | 28 -- .../__snapshots__/Dropdown.test.tsx.snap | 316 ------------------ .../__snapshots__/OptionsMenu.test.tsx.snap | 12 - 3 files changed, 356 deletions(-) diff --git a/packages/react-core/src/components/ApplicationLauncher/__tests__/__snapshots__/ApplicationLauncher.test.tsx.snap b/packages/react-core/src/components/ApplicationLauncher/__tests__/__snapshots__/ApplicationLauncher.test.tsx.snap index 39923fca11a..63c36297977 100644 --- a/packages/react-core/src/components/ApplicationLauncher/__tests__/__snapshots__/ApplicationLauncher.test.tsx.snap +++ b/packages/react-core/src/components/ApplicationLauncher/__tests__/__snapshots__/ApplicationLauncher.test.tsx.snap @@ -581,7 +581,6 @@ exports[`ApplicationLauncher custom icon 1`] = ` } } enterTriggersArrowDown={false} - href="" index={0} isDisabled={false} isHovered={false} @@ -601,7 +600,6 @@ exports[`ApplicationLauncher custom icon 1`] = ` Link @@ -624,7 +622,6 @@ exports[`ApplicationLauncher custom icon 1`] = ` } } enterTriggersArrowDown={false} - href="" index={1} isDisabled={false} isHovered={false} @@ -644,7 +641,6 @@ exports[`ApplicationLauncher custom icon 1`] = `