Skip to content

fix(app-launcher): less aggressive router linking #3995

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.

Sign up for GitHub

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

Merged
redallen merged 6 commits into from
Mar 31, 2020
Merged

fix(app-launcher): less aggressive router linking #3995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
225b869
fix(app-launcher): less aggressive router linking
redallen Mar 30, 2020
7a7871b
support component={<Link />} api
redallen Mar 30, 2020
f1cba6d
remove default empty href
redallen Mar 30, 2020
0ace8d6
update snapshots
redallen Mar 30, 2020
4e08243
improve docs
redallen Mar 30, 2020
0e55836
typo
redallen Mar 30, 2020
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
11 changes: 3 additions & 8 deletions packages/react-core/src/components/ApplicationLauncher/ApplicationLauncherItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,9 @@ export interface ApplicationLauncherItemProps {
tooltip?: React.ReactNode;
/** Additional tooltip props forwarded to the Tooltip component */
tooltipProps?: any;
/** The component that will wrap the item.
* If you need to render a custom component, for example a react router Link component,
* then pass the component here. Example:
* <ApplicationLauncherItem key="router1" component={
* <Link to="/components/alert/">
* <ApplicationLauncherContent>Router link</ApplicationLauncherContent>
* </Link>
* } />
/** A ReactElement to render, or a string to use as the component tag.
* Example: component={<Link to="/components/alert/">Alert</Link>}
* Example: component="button"
*/
component?: React.ReactNode;
/** Flag indicating if the item is favorited */
Expand Down
28 changes: 0 additions & 28 deletions .../components/ApplicationLauncher/__tests__/__snapshots__/ApplicationLauncher.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={0}
isDisabled={false}
isHovered={false}
Expand All @@ -601,7 +600,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
<a
aria-disabled={false}
className="pf-c-app-launcher__menu-item"
href={null}
tabIndex={-1}
>
Link
Expand All @@ -624,7 +622,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={1}
isDisabled={false}
isHovered={false}
Expand All @@ -644,7 +641,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
<button
className="pf-c-app-launcher__menu-item"
disabled={false}
href={null}
tabIndex={-1}
type="button"
>
Expand All @@ -668,7 +664,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={2}
isDisabled={true}
isHovered={false}
Expand All @@ -688,7 +683,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
<a
aria-disabled={true}
className="pf-m-disabled pf-c-app-launcher__menu-item"
href={null}
tabIndex={-1}
>
Disabled Link
Expand All @@ -712,7 +706,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={3}
isDisabled={true}
isHovered={false}
Expand All @@ -732,7 +725,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
<button
className="pf-m-disabled pf-c-app-launcher__menu-item"
disabled={true}
href={null}
tabIndex={-1}
type="button"
>
Expand All @@ -755,7 +747,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={4}
isDisabled={false}
isHovered={false}
Expand All @@ -772,7 +763,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
>
<div
className="pf-c-app-launcher__separator"
href={null}
/>
</li>
</InternalDropdownItem>
Expand All @@ -791,7 +781,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={5}
isDisabled={false}
isHovered={false}
Expand All @@ -811,7 +800,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
<a
aria-disabled={false}
className="pf-c-app-launcher__menu-item"
href={null}
tabIndex={-1}
>
Separated Link
Expand All @@ -834,7 +822,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={6}
isDisabled={false}
isHovered={false}
Expand All @@ -854,7 +841,6 @@ exports[`ApplicationLauncher custom icon 1`] = `
<button
className="pf-c-app-launcher__menu-item"
disabled={false}
href={null}
tabIndex={-1}
type="button"
>
Expand Down Expand Up @@ -2223,7 +2209,6 @@ exports[`ApplicationLauncher expanded 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={0}
isDisabled={false}
isHovered={false}
Expand All @@ -2243,7 +2228,6 @@ exports[`ApplicationLauncher expanded 1`] = `
<a
aria-disabled={false}
className="pf-c-app-launcher__menu-item"
href={null}
tabIndex={-1}
>
Link
Expand All @@ -2266,7 +2250,6 @@ exports[`ApplicationLauncher expanded 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={1}
isDisabled={false}
isHovered={false}
Expand All @@ -2286,7 +2269,6 @@ exports[`ApplicationLauncher expanded 1`] = `
<button
className="pf-c-app-launcher__menu-item"
disabled={false}
href={null}
tabIndex={-1}
type="button"
>
Expand All @@ -2310,7 +2292,6 @@ exports[`ApplicationLauncher expanded 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={2}
isDisabled={true}
isHovered={false}
Expand All @@ -2330,7 +2311,6 @@ exports[`ApplicationLauncher expanded 1`] = `
<a
aria-disabled={true}
className="pf-m-disabled pf-c-app-launcher__menu-item"
href={null}
tabIndex={-1}
>
Disabled Link
Expand All @@ -2354,7 +2334,6 @@ exports[`ApplicationLauncher expanded 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={3}
isDisabled={true}
isHovered={false}
Expand All @@ -2374,7 +2353,6 @@ exports[`ApplicationLauncher expanded 1`] = `
<button
className="pf-m-disabled pf-c-app-launcher__menu-item"
disabled={true}
href={null}
tabIndex={-1}
type="button"
>
Expand All @@ -2397,7 +2375,6 @@ exports[`ApplicationLauncher expanded 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={4}
isDisabled={false}
isHovered={false}
Expand All @@ -2414,7 +2391,6 @@ exports[`ApplicationLauncher expanded 1`] = `
>
<div
className="pf-c-app-launcher__separator"
href={null}
/>
</li>
</InternalDropdownItem>
Expand All @@ -2433,7 +2409,6 @@ exports[`ApplicationLauncher expanded 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={5}
isDisabled={false}
isHovered={false}
Expand All @@ -2453,7 +2428,6 @@ exports[`ApplicationLauncher expanded 1`] = `
<a
aria-disabled={false}
className="pf-c-app-launcher__menu-item"
href={null}
tabIndex={-1}
>
Separated Link
Expand All @@ -2476,7 +2450,6 @@ exports[`ApplicationLauncher expanded 1`] = `
}
}
enterTriggersArrowDown={false}
href=""
index={6}
isDisabled={false}
isHovered={false}
Expand All @@ -2496,7 +2469,6 @@ exports[`ApplicationLauncher expanded 1`] = `
<button
className="pf-c-app-launcher__menu-item"
disabled={false}
href={null}
tabIndex={-1}
type="button"
>
Expand Down
15 changes: 8 additions & 7 deletions ...s/react-core/src/components/ApplicationLauncher/examples/ApplicationLauncher.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -86,24 +88,23 @@ 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 = [
<ApplicationLauncherItem
key="router1"
component={
<Text component="a" href="#" style={exampleStyle}>
Router link
</Text>
<Link to="/" style={exampleStyle}>
Copy link
Contributor

@mcoker mcoker Mar 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is style={exampleStyle} necessary for the example?

@reach/router Link
</Link>
}
/>,
<ApplicationLauncherItem
key="router2"
isExternal
icon={icon}
component={
<Text component="a" href="#" style={exampleStyle}>
<ApplicationLauncherContent>Router link with icon</ApplicationLauncherContent>
</Text>
<Link to="/" style={exampleStyle}>
<ApplicationLauncherContent>@reach/router Link with icon</ApplicationLauncherContent>
</Link>
}
/>,
<ApplicationLauncherItem key="application_1a" href="#">
Expand Down
13 changes: 8 additions & 5 deletions packages/react-core/src/components/Dropdown/DropdownItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ export interface DropdownItemProps extends InternalDropdownItemProps {
className?: string;
/** Class to be applied to list item */
listItemClassName?: string;
/** Indicates which component will be used as dropdown item */
/** A ReactElement to render, or a string to use as the component tag.
* Example: component={<Link to="/components/alert/">Alert</Link>}
* Example: component="button"
*/
component?: React.ReactNode;
/** Variant of the item. The 'icon' variant should use DropdownItemIcon to wrap contained icons or images. */
variant?: 'item' | 'icon';
Expand All @@ -36,7 +39,7 @@ export const DropdownItem: React.FunctionComponent<DropdownItemProps> = ({
variant = 'item',
isDisabled = false,
isHovered = false,
href = '',
href,
tooltip = null,
tooltipProps = {},
listItemClassName,
Expand All @@ -53,8 +56,6 @@ export const DropdownItem: React.FunctionComponent<DropdownItemProps> = ({
context={context}
role="menuitem"
tabIndex={-1}
// eslint-disable-next-line react/no-children-prop
children={children}
className={className}
component={component}
variant={variant}
Expand All @@ -68,7 +69,9 @@ export const DropdownItem: React.FunctionComponent<DropdownItemProps> = ({
additionalChild={additionalChild}
customChild={customChild}
{...props}
/>
>
{children}
</InternalDropdownItem>
)}
</DropdownArrowContext.Consumer>
);
Loading