Skip to content
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.

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

My Jetpack: Fix Protect card Tooltip placement & content issues #40691

Merged
merged 9 commits into from
Dec 21, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@ import { Popover } from '@wordpress/components';
import { useViewportMatch } from '@wordpress/compose';
import { useState, useCallback, useRef } from 'react';
import useAnalytics from '../../hooks/use-analytics';
import type { FC, ReactNode } from 'react';
import type { PopoverProps } from './types';
import type { FC } from 'react';

import './style.scss';

type Props = {
children: ReactNode;
interface Props extends PopoverProps {
className?: string;
icon?: string;
iconSize?: number;
tracksEventName?: string;
tracksEventProps?: Record< Lowercase< string >, unknown >;
};
}

export const InfoTooltip: FC< Props > = ( {
children,
Expand Down
210 changes: 210 additions & 0 deletions projects/packages/my-jetpack/_inc/components/info-tooltip/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
/* eslint-disable jsdoc/check-indentation */
elliottprogrammer marked this conversation as resolved.
Show resolved Hide resolved
import type { ReactNode, MutableRefObject, SyntheticEvent } from 'react';

type PositionYAxis = 'top' | 'middle' | 'bottom';
type PositionXAxis = 'left' | 'center' | 'right';
type PositionCorner = 'top' | 'right' | 'bottom' | 'left';

type DomRectWithOwnerDocument = DOMRect & {
ownerDocument?: Document;
};

type PopoverPlacement =
| 'top'
| 'top-start'
| 'top-end'
| 'right'
| 'right-start'
| 'right-end'
| 'bottom'
| 'bottom-start'
| 'bottom-end'
| 'left'
| 'left-start'
| 'left-end'
| 'overlay';

export type PopoverAnchorRefReference = MutableRefObject< Element | null | undefined >;
export type PopoverAnchorRefTopBottom = { top: Element; bottom: Element };

export type VirtualElement = Pick< Element, 'getBoundingClientRect' > & {
ownerDocument?: Document;
};

export type PopoverProps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing the Popover WordPress component doesn't have an exported types we could use instead? If not, how hard would it be to add them there? Only asking because the ideal scenario would be to have the component type itself, but I understand that may be more difficult than it's worth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah right, the Popover WordPress component doesn't have any exported types, unfortunately. But yeah you're right, it would be nice if it did! 👍
I'll add a followup maintenance task to update the WordPress Popover component to export the Popover type, and then come back here to clean up the InfoTooltip types.

/**
* The name of the Slot in which the popover should be rendered. It should
* be also passed to the corresponding `PopoverSlot` component.
*
* @default 'Popover'
*/
__unstableSlotName?: string;
/**
* The element that should be used by the popover as its anchor. It can either
* be an `Element` or, alternatively, a `VirtualElement` — ie. an object with
* the `getBoundingClientRect()` and the `ownerDocument` properties defined.
*
* **The anchor element should be stored in local state** rather than a
* plain React ref to ensure reactive updating when it changes.
*/
anchor?: Element | VirtualElement | null;
/**
* Whether the popover should animate when opening.
*
* @default true
*/
animate?: boolean;
/**
* The `children` elements rendered as the popover's content.
*/
children: ReactNode;
/**
* Show the popover fullscreen on mobile viewports.
*/
expandOnMobile?: boolean;
/**
* Specifies whether the popover should flip across its axis if there isn't
* space for it in the normal placement.
* When the using a 'top' placement, the popover will switch to a 'bottom'
* placement. When using a 'left' placement, the popover will switch to a
* `right' placement.
* The popover will retain its alignment of 'start' or 'end' when flipping.
*
* @default true
*/
flip?: boolean;
/**
* Determines whether tabbing is constrained to within the popover,
* preventing keyboard focus from leaving the popover content without
* explicit focus elswhere, or whether the popover remains part of the wider
* tab order. If no value is passed, it will be derived from `focusOnMount`.
*
* @default `focusOnMount` !== false
*/
constrainTabbing?: boolean;
/**
* By default, the _first tabbable element_ in the popover will receive focus
* when it mounts. This is the same as setting this prop to `"firstElement"`.
* Specifying a `false` value disables the focus handling entirely (this
* should only be done when an appropriately accessible substitute behavior
* exists).
*
* @default 'firstElement'
*/
focusOnMount?: 'firstElement' | boolean;
/**
* A callback invoked when the focus leaves the opened popover. This should
* only be provided in advanced use-cases when a popover should close under
* specific circumstances (for example, if the new `document.activeElement`
* is content of or otherwise controlling popover visibility).
*
* When not provided, the `onClose` callback will be called instead.
*/
onFocusOutside?: ( event: SyntheticEvent ) => void;
/**
* Used to customize the header text shown when the popover is toggled to
* fullscreen on mobile viewports (see the `expandOnMobile` prop).
*/
headerTitle?: string;
/**
* Used to show/hide the arrow that points at the popover's anchor.
*
* @default true
*/
noArrow?: boolean;
/**
* The distance (in px) between the anchor and the popover.
*/
offset?: number;
/**
* A callback invoked when the popover should be closed.
*/
onClose?: () => void;
/**
* Used to specify the popover's position with respect to its anchor.
*
* @default 'bottom-start'
*/
placement?: PopoverPlacement;
/**
* Legacy way to specify the popover's position with respect to its anchor.
* _Note: this prop is deprecated. Use the `placement` prop instead._
*/
position?:
| `${ PositionYAxis }`
| `${ PositionYAxis } ${ PositionXAxis }`
| `${ PositionYAxis } ${ PositionXAxis } ${ PositionCorner }`;
/**
* Adjusts the size of the popover to prevent its contents from going out of
* view when meeting the viewport edges.
*
* @default true
*/
resize?: boolean;
/**
* Enables the `Popover` to shift in order to stay in view when meeting the
* viewport edges.
*
* @default false
*/
shift?: boolean;
/**
* Specifies the popover's style.
*
* Leave undefined for the default style. Other values are:
* - 'unstyled': The popover is essentially without any visible style, it
* has no background, border, outline or drop shadow, but
* the popover contents are still displayed.
* - 'toolbar': A style that has no elevation, but a high contrast with
* other elements. This is matches the style of the
* `Toolbar` component.
*
* @default undefined
*/
variant?: 'unstyled' | 'toolbar';
/**
* Whether to render the popover inline or within the slot.
*
* @default false
*/
inline?: boolean;
// Deprecated props
/**
* Prevent the popover from flipping and resizing when meeting the viewport
* edges. _Note: this prop is deprecated. Instead, provide use the individual
* `flip` and `resize` props._
*
* @deprecated
*/
__unstableForcePosition?: boolean;
/**
* An object extending a `DOMRect` with an additional optional `ownerDocument`
* property, used to specify a fixed popover position.
*
* @deprecated
*/
anchorRect?: DomRectWithOwnerDocument;
/**
* Used to specify a fixed popover position. It can be an `Element`, a React
* reference to an `element`, an object with a `top` and a `bottom` properties
* (both pointing to elements), or a `range`.
*
* @deprecated
*/
anchorRef?: Element | PopoverAnchorRefReference | PopoverAnchorRefTopBottom | Range;
/**
* A function returning the same value as the one expected by the `anchorRect`
* prop, used to specify a dynamic popover position.
*
* @deprecated
*/
getAnchorRect?: ( fallbackReferenceElement: Element | null ) => DomRectWithOwnerDocument;
/**
* Used to enable a different visual style for the popover.
* _Note: this prop is deprecated. Use the `variant` prop with the
* 'toolbar' value instead._
*
* @deprecated
*/
isAlternate?: boolean;
};
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useViewportMatch } from '@wordpress/compose';
import { __ } from '@wordpress/i18n';
import clsx from 'clsx';
import useProduct from '../../../data/products/use-product';
Expand Down Expand Up @@ -41,6 +42,7 @@ export const AutoFirewallStatus = () => {
*/
function WafStatus( { status }: { status: 'active' | 'inactive' | 'off' } ) {
const slug = 'protect';
const isMobileViewport: boolean = useViewportMatch( 'medium', '<' );
const { detail } = useProduct( slug );
const { hasPaidPlanForProduct = false } = detail || {};
const tooltipContent = useProtectTooltipCopy();
Expand Down Expand Up @@ -78,11 +80,10 @@ function WafStatus( { status }: { status: 'active' | 'inactive' | 'off' } ) {
feature: 'jetpack-protect',
has_paid_plan: hasPaidPlanForProduct,
} }
placement={ isMobileViewport ? 'top' : 'right' }
>
<>
<h3>{ autoFirewallTooltip.title }</h3>
<p>{ autoFirewallTooltip.text }</p>
</>
<h3>{ autoFirewallTooltip.title }</h3>
<p>{ autoFirewallTooltip.text }</p>
</InfoTooltip>
</div>
<div className="value-section__data">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,8 @@ function BlockedStatus( { status }: { status: 'active' | 'inactive' | 'off' } )
message: 'no data yet',
} }
>
<>
<h3>{ blockedLoginsTooltip.title }</h3>
<p>{ blockedLoginsTooltip.text }</p>
</>
<h3>{ blockedLoginsTooltip.title }</h3>
<p>{ blockedLoginsTooltip.text }</p>
</InfoTooltip>
</div>
<div className="value-section__data">
Expand Down Expand Up @@ -113,10 +111,8 @@ function BlockedStatus( { status }: { status: 'active' | 'inactive' | 'off' } )
status: status,
} }
>
<>
<h3>{ blockedLoginsTooltip.title }</h3>
<p>{ blockedLoginsTooltip.text }</p>
</>
<h3>{ blockedLoginsTooltip.title }</h3>
<p>{ blockedLoginsTooltip.text }</p>
</InfoTooltip>
</div>
<div className="value-section__data">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@ const ProtectValueSection = () => {
status: 'inactive',
} }
>
<>
<h3>{ pluginsThemesTooltip.title }</h3>
<p>{ pluginsThemesTooltip.text }</p>
</>
<h3>{ pluginsThemesTooltip.title }</h3>
<p>{ pluginsThemesTooltip.text }</p>
</InfoTooltip>
) }
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ function ThreatStatus( {
focusOnMount={ 'container' }
onClose={ hideTooltip }
>
<>
<div className="info-tooltip__content">
<h3>{ scanThreatsTooltip.title }</h3>
<p>{ scanThreatsTooltip.text }</p>
</>
</div>
</Popover>
) }
</div>
Expand All @@ -157,8 +157,20 @@ function ThreatStatus( {

return (
<>
<div className={ baseStyles.valueSectionHeading }>
<div className={ clsx( baseStyles.valueSectionHeading, 'value-section__heading' ) }>
{ __( 'Threats', 'jetpack-my-jetpack' ) }
<InfoTooltip
tracksEventName={ 'protect_card_tooltip_open' }
tracksEventProps={ {
location: 'threats',
feature: 'jetpack-protect',
has_paid_plan: true,
threats: numThreats,
} }
>
<h3>{ scanThreatsTooltip.title }</h3>
<p>{ scanThreatsTooltip.text }</p>
</InfoTooltip>
</div>
<div className="value-section__data">
<div className="scan-threats__threat-count">{ numThreats }</div>
Expand Down Expand Up @@ -213,10 +225,8 @@ function ScanStatus( { status }: { status: 'success' | 'partial' | 'off' } ) {
threats: 0,
} }
>
<>
<h3>{ scanThreatsTooltip.title }</h3>
<p>{ scanThreatsTooltip.text }</p>
</>
<h3>{ scanThreatsTooltip.title }</h3>
<p>{ scanThreatsTooltip.text }</p>
</InfoTooltip>
</div>
<div className="value-section__data">
Expand Down
Loading
Loading