From 7a4a25186c4c13033bff1de66fe1e6a261b72600 Mon Sep 17 00:00:00 2001 From: Nate Weller Date: Mon, 28 Oct 2024 14:24:22 -0600 Subject: [PATCH] Use inline button for primary fixer action --- .../components/threat-fixer-button/index.tsx | 137 ++++++++++++++++ .../stories/index.stories.tsx | 30 ++++ .../threat-fixer-button/styles.module.scss | 9 ++ .../components/threat-fixer-button/utils.ts | 17 ++ .../threats-data-views/fixer-status.tsx | 137 ---------------- .../components/threats-data-views/index.tsx | 149 +++++++++--------- .../stories/index.stories.tsx | 9 ++ .../threats-data-views/styles.module.scss | 98 +----------- .../components/threats-data-views/utils.ts | 54 +------ projects/js-packages/components/index.ts | 1 + 10 files changed, 289 insertions(+), 352 deletions(-) create mode 100644 projects/js-packages/components/components/threat-fixer-button/index.tsx create mode 100644 projects/js-packages/components/components/threat-fixer-button/stories/index.stories.tsx create mode 100644 projects/js-packages/components/components/threat-fixer-button/styles.module.scss create mode 100644 projects/js-packages/components/components/threat-fixer-button/utils.ts delete mode 100644 projects/js-packages/components/components/threats-data-views/fixer-status.tsx diff --git a/projects/js-packages/components/components/threat-fixer-button/index.tsx b/projects/js-packages/components/components/threat-fixer-button/index.tsx new file mode 100644 index 0000000000000..9994bae2116ee --- /dev/null +++ b/projects/js-packages/components/components/threat-fixer-button/index.tsx @@ -0,0 +1,137 @@ +import { Button, Text, ActionPopover } from '@automattic/jetpack-components'; +import { Threat } from '@automattic/jetpack-scan'; +import { ExternalLink } from '@wordpress/components'; +import { createInterpolateElement, useCallback, useMemo, useState } from '@wordpress/element'; +import { __, sprintf } from '@wordpress/i18n'; +import { PAID_PLUGIN_SUPPORT_URL } from '../threats-data-views/constants'; +import styles from './styles.module.scss'; +import { fixerStatusIsStale } from './utils'; + +/** + * ThreatFixerButton + * + * @param {object} props - Component props. + * @param {object} props.threat - The threat. + * @param {Function} props.onClick - The onClick function. + * @param {string} props.className - The className. + * + * @return {JSX.Element} The component. + */ +export default function ThreatFixerButton( { + threat, + className, + onClick, +}: { + threat: Threat; + className?: string; + onClick: ( items: Threat[] ) => void; +} ): JSX.Element { + const [ isPopoverVisible, setIsPopoverVisible ] = useState( false ); + + const [ anchor, setAnchor ] = useState( null ); + + const children = useMemo( () => { + if ( ! threat.fixable ) { + return null; + } + if ( threat.fixer && threat.fixer.error ) { + return __( 'Error', 'jetpack' ); + } + if ( threat.fixer && threat.fixer.status === 'in_progress' ) { + return __( 'Fixing…', 'jetpack' ); + } + if ( threat.fixable.fixer === 'delete' ) { + return __( 'Delete', 'jetpack' ); + } + if ( threat.fixable.fixer === 'update' ) { + return __( 'Update', 'jetpack' ); + } + return __( 'Fix', 'jetpack' ); + }, [ threat.fixable, threat.fixer ] ); + + const errorMessage = useMemo( () => { + if ( threat.fixer && fixerStatusIsStale( threat.fixer ) ) { + return __( 'Fixer is taking longer than expected.', 'jetpack' ); + } + + if ( threat.fixer && 'error' in threat.fixer && threat.fixer.error ) { + return __( 'An error occurred auto-fixing this threat.', 'jetpack' ); + } + + return null; + }, [ threat.fixer ] ); + + const handleClick = useCallback( + ( event: React.MouseEvent ) => { + event.stopPropagation(); + if ( errorMessage && ! isPopoverVisible ) { + setIsPopoverVisible( true ); + return; + } + onClick( [ threat ] ); + }, + [ onClick, errorMessage, isPopoverVisible, threat ] + ); + + const closePopover = useCallback( () => { + setIsPopoverVisible( false ); + }, [] ); + + if ( ! threat.fixable ) { + return null; + } + + return ( +
+
+ ); +} diff --git a/projects/js-packages/components/components/threat-fixer-button/stories/index.stories.tsx b/projects/js-packages/components/components/threat-fixer-button/stories/index.stories.tsx new file mode 100644 index 0000000000000..6b378030a2d89 --- /dev/null +++ b/projects/js-packages/components/components/threat-fixer-button/stories/index.stories.tsx @@ -0,0 +1,30 @@ +import ThreatFixerButton from '../index.js'; + +export default { + title: 'JS Packages/Components/Threat Fixer Button', + component: ThreatFixerButton, +}; + +export const Default = args => ; +Default.args = { + threat: { fixable: { fixer: 'edit' } }, + onClick: () => alert( 'Edit fixer callback triggered' ), // eslint-disable-line no-alert +}; + +export const Update = args => ; +Update.args = { + threat: { fixable: { fixer: 'update' } }, + onClick: () => alert( 'Update fixer callback triggered' ), // eslint-disable-line no-alert +}; + +export const Delete = args => ; +Delete.args = { + threat: { fixable: { fixer: 'delete' } }, + onClick: () => alert( 'Delete fixer callback triggered' ), // eslint-disable-line no-alert +}; + +export const Loading = args => ; +Loading.args = { + threat: { fixable: { fixer: 'edit' }, fixer: { status: 'in_progress' } }, + onClick: () => alert( 'Fixer callback triggered' ), // eslint-disable-line no-alert +}; diff --git a/projects/js-packages/components/components/threat-fixer-button/styles.module.scss b/projects/js-packages/components/components/threat-fixer-button/styles.module.scss new file mode 100644 index 0000000000000..071761daff049 --- /dev/null +++ b/projects/js-packages/components/components/threat-fixer-button/styles.module.scss @@ -0,0 +1,9 @@ +.support-link { + color: inherit; + + &:focus, + &:hover { + color: inherit; + box-shadow: none; + } +} diff --git a/projects/js-packages/components/components/threat-fixer-button/utils.ts b/projects/js-packages/components/components/threat-fixer-button/utils.ts new file mode 100644 index 0000000000000..76c1655937bfb --- /dev/null +++ b/projects/js-packages/components/components/threat-fixer-button/utils.ts @@ -0,0 +1,17 @@ +import { ThreatFixStatus } from '@automattic/jetpack-scan'; + +const FIXER_IS_STALE_THRESHOLD = 1000 * 60 * 60 * 24; // 24 hours + +export const fixerTimestampIsStale = ( lastUpdatedTimestamp: string ) => { + const now = new Date(); + const lastUpdated = new Date( lastUpdatedTimestamp ); + return now.getTime() - lastUpdated.getTime() >= FIXER_IS_STALE_THRESHOLD; +}; + +export const fixerStatusIsStale = ( fixerStatus: ThreatFixStatus ) => { + return ( + 'status' in fixerStatus && + fixerStatus.status === 'in_progress' && + fixerTimestampIsStale( fixerStatus.last_updated ) + ); +}; diff --git a/projects/js-packages/components/components/threats-data-views/fixer-status.tsx b/projects/js-packages/components/components/threats-data-views/fixer-status.tsx deleted file mode 100644 index 3e9f5b5dd1efe..0000000000000 --- a/projects/js-packages/components/components/threats-data-views/fixer-status.tsx +++ /dev/null @@ -1,137 +0,0 @@ -import { type ThreatFixStatus } from '@automattic/jetpack-scan'; -import { ExternalLink, Spinner } from '@wordpress/components'; -import { createInterpolateElement } from '@wordpress/element'; -import { __, sprintf } from '@wordpress/i18n'; -import { Icon } from '@wordpress/icons'; -import { check } from '@wordpress/icons'; -import IconTooltip from '../icon-tooltip'; -import Text from '../text'; -import { PAID_PLUGIN_SUPPORT_URL } from './constants'; -import styles from './styles.module.scss'; -import { fixerStatusIsStale } from './utils'; - -/** - * InfoIconTooltip component. - * - * @param {object} props - Component props. - * @param {boolean} props.message - The popover message. - * @param {object} props.size - The size of the icon. - * - * @return {JSX.Elenment} The component. - */ -export function InfoIconTooltip( { - message, - size = 20, -}: { - message?: string; - size?: number; -} ): JSX.Element { - return ( - - - { createInterpolateElement( - sprintf( - /* translators: %s: Number of hide items */ - __( '%s Please try again or contact support.', 'jetpack' ), - message - ), - { - supportLink: ( - - ), - } - ) } - - - ); -} - -/** - * Fixer Status component. - * - * @param {object} props - Component props. - * @param {boolean} props.fixer - The fixer status. - * - * @return {JSX.Element} The component. - */ -export default function FixerStatusIcon( { fixer }: { fixer?: ThreatFixStatus } ): JSX.Element { - if ( fixer && fixerStatusIsStale( fixer ) ) { - return ( - - ); - } - - if ( fixer && 'error' in fixer && fixer.error ) { - return ( - - ); - } - - if ( fixer && 'status' in fixer && fixer.status === 'in_progress' ) { - return ( -
- -
- ); - } - - return ; -} - -/** - * FixerStatusText component. - * - * @param {object} props - Component props. - * @param {boolean} props.fixer - The fixer status. - * - * @return {JSX.Element} The component. - */ -function FixerStatusText( { fixer }: { fixer?: ThreatFixStatus } ): JSX.Element { - if ( fixer && fixerStatusIsStale( fixer ) ) { - return ( - - { __( 'Fixer is taking longer than expected', 'jetpack' ) } - - ); - } - - if ( fixer && 'error' in fixer && fixer.error ) { - return ( - - { __( 'An error occurred auto-fixing this threat', 'jetpack' ) } - - ); - } - - if ( fixer && 'status' in fixer && fixer.status === 'in_progress' ) { - return { __( 'Auto-fixing', 'jetpack' ) }; - } - - return { __( 'Auto-fixable', 'jetpack' ) }; -} - -/** - * FixerStatusBadge component. - * - * @param {object} props - Component props. - * @param {boolean} props.fixer - The fixer status. - * - * @return {string} The component. - */ -export function FixerStatusBadge( { fixer }: { fixer?: ThreatFixStatus } ): JSX.Element { - return ( -
- - -
- ); -} diff --git a/projects/js-packages/components/components/threats-data-views/index.tsx b/projects/js-packages/components/components/threats-data-views/index.tsx index beb94c8a3b4c9..a08719ecaa693 100644 --- a/projects/js-packages/components/components/threats-data-views/index.tsx +++ b/projects/js-packages/components/components/threats-data-views/index.tsx @@ -16,11 +16,11 @@ import { __ } from '@wordpress/i18n'; import { Icon } from '@wordpress/icons'; import { useCallback, useMemo, useState } from 'react'; import Badge from '../badge'; +import ThreatFixerButton from '../threat-fixer-button'; import ThreatSeverityBadge from '../threat-severity-badge'; import { THREAT_STATUSES, THREAT_TYPES } from './constants'; -import FixerStatusIcon, { FixerStatusBadge } from './fixer-status'; import styles from './styles.module.scss'; -import { getThreatIcon, getThreatSubtitle, getThreatType } from './utils'; +import { getThreatIcon, getThreatType } from './utils'; /** * DataViews component for displaying security threats. @@ -55,7 +55,7 @@ export default function ThreatsDataViews( { isThreatEligibleForFix?: ( threat: Threat ) => boolean; isThreatEligibleForIgnore?: ( threat: Threat ) => boolean; isThreatEligibleForUnignore?: ( threat: Threat ) => boolean; - onFixThreats?: ActionButton< Threat >[ 'callback' ]; + onFixThreats?: ( threats: Threat[] ) => void; onIgnoreThreats?: ActionButton< Threat >[ 'callback' ]; onUnignoreThreats?: ActionButton< Threat >[ 'callback' ]; } ): JSX.Element { @@ -80,14 +80,22 @@ export default function ThreatsDataViews( { const defaultLayouts: SupportedLayouts = { table: { ...baseView, - fields: [ 'severity', 'threat', 'auto-fix' ], + fields: [ 'severity', 'threat', 'type', 'fix' ], layout: { primaryField: 'severity', + combinedFields: [ + { + id: 'threat', + label: __( 'Threat', 'jetpack' ), + children: [ 'title', 'description' ], + direction: 'vertical', + }, + ], }, }, list: { ...baseView, - fields: [ 'severity', 'subtitle', 'signature', 'auto-fix' ], + fields: [ 'severity', 'type', 'signature' ], layout: { primaryField: 'title', mediaField: 'icon', @@ -167,38 +175,42 @@ export default function ThreatsDataViews( { const fields = useMemo( () => { const result: Field< Threat >[] = [ { - id: 'threat', - label: __( 'Threat', 'jetpack' ), + id: 'title', + label: __( 'Title', 'jetpack' ), enableGlobalSearch: true, enableHiding: false, - getValue( { item }: { item: Threat } ) { - return item.title + item.description; - }, - render( { item }: { item: Threat } ) { - return ( -
-
- - { getThreatSubtitle( item ) } -
-
{ item.title }
-
{ item.description }
-
- ); - }, + render: ( { item }: { item: Threat } ) => ( +
{ item.title }
+ ), }, { - id: 'title', - label: __( 'Title', 'jetpack' ), + id: 'description', + label: __( 'Description', 'jetpack' ), enableGlobalSearch: true, enableHiding: false, + render: ( { item }: { item: Threat } ) => ( +
{ item.description }
+ ), }, { id: 'icon', label: __( 'Icon', 'jetpack' ), enableHiding: false, getValue( { item }: { item: Threat } ) { - return getThreatType( item ); + if ( item.signature === 'Vulnerable.WP.Core' ) { + return 'core'; + } + if ( item.extension ) { + return item.extension.type; + } + if ( item.filename ) { + return 'file'; + } + if ( item.table ) { + return 'database'; + } + + return ''; }, render( { item }: { item: Threat } ) { return ( @@ -208,6 +220,21 @@ export default function ThreatsDataViews( { ); }, }, + ...( dataFields.includes( 'severity' ) + ? [ + { + id: 'severity', + label: __( 'Severity', 'jetpack' ), + type: 'integer' as FieldType, + getValue( { item }: { item: Threat } ) { + return item.severity ?? 0; + }, + render( { item }: { item: Threat } ) { + return ; + }, + }, + ] + : [] ), { id: 'status', label: __( 'Status', 'jetpack' ), @@ -241,31 +268,10 @@ export default function ThreatsDataViews( { }, { id: 'type', - label: __( 'Category', 'jetpack' ), + label: __( 'Type', 'jetpack' ), elements: THREAT_TYPES, getValue( { item }: { item: Threat } ) { - if ( item.signature === 'Vulnerable.WP.Core' ) { - return 'core'; - } - if ( item.extension ) { - return item.extension.type; - } - if ( item.filename ) { - return 'file'; - } - if ( item.table ) { - return 'database'; - } - - return 'uncategorized'; - }, - }, - { - id: 'subtitle', - label: __( 'Affected Item', 'jetpack' ), - enableHiding: false, - getValue( { item }: { item: Threat } ) { - return getThreatSubtitle( item ); + return getThreatType( item ) ?? ''; }, }, ...( dataFields.includes( 'signature' ) @@ -281,45 +287,40 @@ export default function ThreatsDataViews( { }, ] : [] ), - ...( dataFields.includes( 'severity' ) + + ...( dataFields.includes( 'fixable' ) ? [ { - id: 'severity', - label: __( 'Severity', 'jetpack' ), - type: 'integer' as FieldType, + id: 'auto-fix', + label: __( 'Auto-fixable', 'jetpack' ), + enableHiding: false, + elements: [ + { + value: 'yes', + label: __( 'Yes', 'jetpack' ), + }, + { + value: 'no', + label: __( 'No', 'jetpack' ), + }, + ], getValue( { item }: { item: Threat } ) { - return item.severity ?? 0; - }, - render( { item }: { item: Threat } ) { - return ; + return item.fixable ? 'yes' : 'no'; }, }, - ] - : [] ), - ...( dataFields.includes( 'fixable' ) - ? [ { - id: 'auto-fix', - label: __( 'Auto-fix', 'jetpack' ), + id: 'fix', + label: __( 'Fix', 'jetpack' ), enableHiding: false, - type: 'integer' as FieldType, getValue( { item }: { item: Threat } ) { - return item.fixable ? 1 : 0; + return item.fixable ? 'yes' : 'no'; }, render( { item }: { item: Threat } ) { if ( ! item.fixable ) { return null; } - if ( view.type === 'table' ) { - return ( -
- -
- ); - } - - return ; + return ; }, }, ] @@ -365,7 +366,7 @@ export default function ThreatsDataViews( { ]; return result; - }, [ extensions, signatures, dataFields, view ] ); + }, [ dataFields, extensions, signatures, onFixThreats ] ); /** * DataView actions - collection of operations that can be performed upon each record. diff --git a/projects/js-packages/components/components/threats-data-views/stories/index.stories.tsx b/projects/js-packages/components/components/threats-data-views/stories/index.stories.tsx index 522b4c4b075d5..4693c94fdd093 100644 --- a/projects/js-packages/components/components/threats-data-views/stories/index.stories.tsx +++ b/projects/js-packages/components/components/threats-data-views/stories/index.stories.tsx @@ -286,6 +286,15 @@ FixerStatuses.args = { source: null, }, ], + onFixThreats: () => + alert( 'Threat fix action callback triggered! This is handled by the component consumer.' ), // eslint-disable-line no-alert + onIgnoreThreats: () => + alert( 'Ignore threat action callback triggered! This is handled by the component consumer.' ), // eslint-disable-line no-alert + onUnignoreThreats: () => + // eslint-disable-next-line no-alert + alert( + 'Unignore threat action callback triggered! This is handled by the component consumer.' + ), }; export const FreeResults = args => ; diff --git a/projects/js-packages/components/components/threats-data-views/styles.module.scss b/projects/js-packages/components/components/threats-data-views/styles.module.scss index 4248f49871d5a..98789b2f794ec 100644 --- a/projects/js-packages/components/components/threats-data-views/styles.module.scss +++ b/projects/js-packages/components/components/threats-data-views/styles.module.scss @@ -1,33 +1,15 @@ @import '@wordpress/dataviews/build-style/style.css'; -.threat__primary { - display: flex; - flex-direction: column; - gap: 4px; - white-space: initial; -} - -.threat__subtitle { - display: flex; - align-items: center; - gap: 6px; - font-size: 12px; - color: var( --jp-gray-80 ); - margin-bottom: 4px; - - > svg { - color: currentColor; - } -} - .threat__title { color: var( --jp-gray-80 ); font-weight: 510; + white-space: initial; } .threat__description { color: var( --jp-gray-80 ); font-size: 12px; + white-space: initial; } .threat__fixedOn, @@ -39,90 +21,16 @@ color: var( --jp-green-70 ); } - .threat__media { width: 100%; height: 100%; display: flex; align-items: center; justify-content: center; - color: black; background-color: #EDFFEE; border-color: #EDFFEE; svg { - fill: currentColor; - } -} - -/** - * Auto-fix status icons - */ - - .threat__fixer { - min-width: 54px; - display: flex; - justify-content: center; -} - - -.fixer-status { - display: flex; - align-items: center; - line-height: 0; - - .icon-spinner { - margin-left: 1px; - } - - .icon-info { - margin-left: -3px; - } - - .icon-check { - margin-left: -6px; - } -} - -.icon-check { - fill: var( --jp-green-40 ); -} - -.icon-tooltip { - width: fit-content; -} - -.icon-tooltip__container { - text-align: left; - height: 20px; -} - -.icon-tooltip__icon { - color: var( --jp-red ); -} - -.icon-spinner svg { - margin: 0; -} - -.spinner-spacer { - margin-left: 8px; -} - -.info-spacer { - margin-left: 4px; -} - -.check-spacer { - margin-left: -2px; -} - -.support-link { - color: inherit; - - &:focus, - &:hover { - color: inherit; - box-shadow: none; + fill: black; } } diff --git a/projects/js-packages/components/components/threats-data-views/utils.ts b/projects/js-packages/components/components/threats-data-views/utils.ts index f8c3151ea35b4..65f244f69d2db 100644 --- a/projects/js-packages/components/components/threats-data-views/utils.ts +++ b/projects/js-packages/components/components/threats-data-views/utils.ts @@ -1,25 +1,6 @@ -import { type Threat, type ThreatFixStatus } from '@automattic/jetpack-scan'; +import { type Threat } from '@automattic/jetpack-scan'; import { code, color, grid, plugins, shield, wordpress } from '@wordpress/icons'; -export const getThreatIcon = ( threat: Threat ) => { - const type = getThreatType( threat ); - - switch ( type ) { - case 'plugin': - return plugins; - case 'theme': - return color; - case 'core': - return wordpress; - case 'file': - return code; - case 'database': - return grid; - default: - return shield; - } -}; - export const getThreatType = ( threat: Threat ) => { if ( threat.signature === 'Vulnerable.WP.Core' ) { return 'core'; @@ -37,40 +18,21 @@ export const getThreatType = ( threat: Threat ) => { return null; }; -export const getThreatSubtitle = ( threat: Threat ) => { +export const getThreatIcon = ( threat: Threat ) => { const type = getThreatType( threat ); switch ( type ) { case 'plugin': + return plugins; case 'theme': - return `${ threat.extension?.name } (${ threat.extension?.version })`; + return color; case 'core': - return 'WordPress Core'; + return wordpress; case 'file': - // Trim leading slash - if ( threat.filename.startsWith( '/' ) ) { - return threat.filename.slice( 1 ); - } - return threat.filename; + return code; case 'database': - return threat.table; + return grid; default: - return ''; + return shield; } }; - -const FIXER_IS_STALE_THRESHOLD = 1000 * 60 * 60 * 24; // 24 hours - -export const fixerTimestampIsStale = ( lastUpdatedTimestamp: string ) => { - const now = new Date(); - const lastUpdated = new Date( lastUpdatedTimestamp ); - return now.getTime() - lastUpdated.getTime() >= FIXER_IS_STALE_THRESHOLD; -}; - -export const fixerStatusIsStale = ( fixerStatus: ThreatFixStatus ) => { - return ( - 'status' in fixerStatus && - fixerStatus.status === 'in_progress' && - fixerTimestampIsStale( fixerStatus.last_updated ) - ); -}; diff --git a/projects/js-packages/components/index.ts b/projects/js-packages/components/index.ts index 3cfdd7aa00065..eb90df97ad5fe 100644 --- a/projects/js-packages/components/index.ts +++ b/projects/js-packages/components/index.ts @@ -44,6 +44,7 @@ export { default as CopyToClipboard } from './components/copy-to-clipboard'; export * from './components/icons'; export { default as SplitButton } from './components/split-button'; export { default as ThemeProvider } from './components/theme-provider'; +export { default as ThreatFixerButton } from './components/threat-fixer-button'; export { default as ThreatSeverityBadge } from './components/threat-severity-badge'; export { default as ThreatsDataViews } from './components/threats-data-views'; export { default as Text, H2, H3, Title } from './components/text';