Fix style of action item that is not a link

Links need empty href to be styled properly.
Rewrite component to function component.

shared/src/actions/__snapshots__/ActionItem.test.tsx.snap

@@ -3,8 +3,9 @@
 exports[`ActionItem actionItem variant 1`] = `
 <a
   aria-label="d"
-  className="action-item action-item--variant-action-item "
+  className="action-item action-item--variant-action-item"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -24,8 +25,9 @@ exports[`ActionItem actionItem variant 1`] = `
 exports[`ActionItem non-actionItem variant 1`] = `
 <a
   aria-label="d"
-  className="action-item "
+  className="action-item"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -45,7 +47,8 @@ exports[`ActionItem non-actionItem variant 1`] = `
 exports[`ActionItem non-pressed actionItem 1`] = `
 <a
   aria-pressed={false}
-  className="action-item action-item--variant-action-item "
+  className="action-item action-item--variant-action-item"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -75,7 +78,8 @@ exports[`ActionItem noop command 1`] = `
 exports[`ActionItem pressed toggle actionItem 1`] = `
 <a
   aria-pressed={true}
-  className="action-item action-item--variant-action-item action-item--pressed "
+  className="action-item action-item--variant-action-item action-item--pressed"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -89,7 +93,7 @@ exports[`ActionItem pressed toggle actionItem 1`] = `
 
 exports[`ActionItem render as link for "open" command 1`] = `
 <a
-  className="action-item "
+  className="action-item"
   href="https://example.com/bar"
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -103,7 +107,7 @@ exports[`ActionItem render as link for "open" command 1`] = `
 
 exports[`ActionItem render as link with icon for "open" command with different origin 1`] = `
 <a
-  className="action-item "
+  className="action-item"
   href="https://other.com/foo"
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -121,6 +125,7 @@ exports[`ActionItem run command 1`] = `
   aria-label="d"
   className="action-item action-item--variant-action-item disabled"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -140,8 +145,9 @@ exports[`ActionItem run command 1`] = `
 exports[`ActionItem run command 2`] = `
 <a
   aria-label="d"
-  className="action-item action-item--variant-action-item "
+  className="action-item action-item--variant-action-item"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -161,8 +167,9 @@ exports[`ActionItem run command 2`] = `
 exports[`ActionItem run command with error 1`] = `
 <a
   aria-label="d"
-  className="action-item action-item--variant-action-item "
+  className="action-item action-item--variant-action-item"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -182,8 +189,9 @@ exports[`ActionItem run command with error 1`] = `
 exports[`ActionItem run command with error with showInlineError 1`] = `
 <a
   aria-label="Error: x"
-  className="action-item action-item--variant-action-item "
+  className="action-item action-item--variant-action-item"
   data-tooltip="Error: x"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -205,6 +213,7 @@ exports[`ActionItem run command with showLoadingSpinnerDuringExecution 1`] = `
   aria-label="d"
   className="action-item action-item--loading action-item--variant-action-item disabled"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -231,8 +240,9 @@ exports[`ActionItem run command with showLoadingSpinnerDuringExecution 1`] = `
 exports[`ActionItem run command with showLoadingSpinnerDuringExecution 2`] = `
 <a
   aria-label="d"
-  className="action-item action-item--variant-action-item "
+  className="action-item action-item--variant-action-item"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -252,8 +262,9 @@ exports[`ActionItem run command with showLoadingSpinnerDuringExecution 2`] = `
 exports[`ActionItem title element 1`] = `
 <a
   aria-label="d"
-  className="action-item action-item--variant-action-item "
+  className="action-item action-item--variant-action-item"
   data-tooltip="d"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}

shared/src/components/LinkOrButton.tsx

@@ -1,7 +1,12 @@
 import * as H from 'history'
-import * as React from 'react'
+import React, { useCallback } from 'react'
 import { Key } from 'ts-key-enum'
 import { Link } from './Link'
+import classNames from 'classnames'
+import { noop } from 'lodash'
+
+const isSelectKeyPress = (event: React.KeyboardEvent): boolean =>
+    event.key === Key.Enter && !event.ctrlKey && !event.shiftKey && !event.metaKey && !event.altKey
 
 interface Props {
     /** The link destination URL. */
@@ -41,58 +46,62 @@ interface Props {
  * It is keyboard accessible: unlike <Link> or <a>, pressing the enter key triggers it. Unlike
  * <button>, it shows a focus ring.
  */
-export class LinkOrButton extends React.PureComponent<Props> {
-    public render(): JSX.Element | null {
-        const className = `${this.props.className === undefined ? 'nav-link' : this.props.className} ${
-            this.props.disabled ? 'disabled' : ''
-        }`
-
-        const commonProps: React.AnchorHTMLAttributes<HTMLAnchorElement> & {
-            'data-tooltip': string | undefined
-            onAuxClick?: React.MouseEventHandler<HTMLAnchorElement>
-        } = {
-            className,
-            'data-tooltip': this.props['data-tooltip'],
-            'aria-label': this.props['data-tooltip'],
-            role: typeof this.props.pressed === 'boolean' ? 'button' : undefined,
-            'aria-pressed': this.props.pressed,
-            tabIndex: 0,
-            onClick: this.onAnchorClick,
-            onKeyPress: this.onAnchorKeyPress,
-        }
+export const LinkOrButton: React.FunctionComponent<Props> = ({
+    className = 'nav-link',
+    to,
+    target,
+    disabled,
+    pressed,
+    'data-tooltip': tooltip,
+    onSelect = noop,
+    children,
+}) => {
+    // We need to set up a keypress listener because <a onclick> doesn't get
+    // triggered by enter.
+    const onAnchorKeyPress: React.KeyboardEventHandler<HTMLAnchorElement> = useCallback(
+        event => {
+            if (isSelectKeyPress(event)) {
+                onSelect(event)
+            }
+        },
+        [onSelect]
+    )
 
-        if (!this.props.to) {
-            // Use onAuxClick so that middle-clicks are caught.
-            commonProps.onAuxClick = this.onAnchorClick
+    const commonProps: React.AnchorHTMLAttributes<HTMLAnchorElement> & {
+        'data-tooltip': string | undefined
+    } = {
+        className: classNames(className, disabled && 'disabled'),
+        'data-tooltip': tooltip,
+        'aria-label': tooltip,
+        role: typeof pressed === 'boolean' ? 'button' : undefined,
+        'aria-pressed': pressed,
+        tabIndex: 0,
+        onClick: onSelect,
+        onKeyPress: onAnchorKeyPress,
+    }
 
-            // Render using an <a> with no href, so that we get a focus ring (when using Bootstrap).
-            // We need to set up a keypress listener because <a onclick> doesn't get triggered by
-            // enter.
-            return <a {...commonProps}>{this.props.children}</a>
-        }
+    const onClickPreventDefault: React.MouseEventHandler<HTMLAnchorElement> = useCallback(
+        event => {
+            // Prevent default action of reloading page because of empty href
+            event.preventDefault()
+            onSelect(event)
+        },
+        [onSelect]
+    )
 
+    if (!to) {
         return (
-            <Link {...commonProps} to={this.props.to} target={this.props.target}>
-                {this.props.children}
-            </Link>
+            // Need empty href for styling reasons
+            // Use onAuxClick so that middle-clicks are caught.
+            <a href="" {...commonProps} onClick={onClickPreventDefault} onAuxClick={onClickPreventDefault}>
+                {children}
+            </a>
         )
     }
 
-    private onAnchorClick: React.MouseEventHandler<HTMLAnchorElement> = e => {
-        if (this.props.onSelect) {
-            this.props.onSelect(e)
-        }
-    }
-
-    private onAnchorKeyPress: React.KeyboardEventHandler<HTMLAnchorElement> = e => {
-        if (isSelectKeyPress(e)) {
-            if (this.props.onSelect) {
-                this.props.onSelect(e)
-            }
-        }
-    }
-}
-
-function isSelectKeyPress(e: React.KeyboardEvent): boolean {
-    return e.key === Key.Enter && !e.ctrlKey && !e.shiftKey && !e.metaKey && !e.altKey
+    return (
+        <Link {...commonProps} to={to} target={target}>
+            {children}
+        </Link>
+    )
 }

shared/src/components/__snapshots__/LinkOrButton.test.tsx.snap

@@ -2,7 +2,8 @@
 
 exports[`LinkOrButton render a button when "to" is undefined 1`] = `
 <a
-  className="nav-link "
+  className="nav-link"
+  href=""
   onAuxClick={[Function]}
   onClick={[Function]}
   onKeyPress={[Function]}
@@ -14,7 +15,7 @@ exports[`LinkOrButton render a button when "to" is undefined 1`] = `
 
 exports[`LinkOrButton render a link when "to" is set 1`] = `
 <a
-  className="nav-link "
+  className="nav-link"
   href="http://example.com"
   onClick={[Function]}
   onKeyPress={[Function]}