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

[Uptime] Fix: Adjust tooltip on Run test button for Private Locations #138863

Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { useCallback, useContext, useState, useEffect } from 'react';
import React, { useCallback, useContext, useState, useEffect, useRef } from 'react';
import { useParams, Redirect } from 'react-router-dom';
import {
EuiFlexGroup,
Expand Down Expand Up @@ -38,6 +38,11 @@ import { monitorManagementListSelector } from '../../../state/selectors';

import { kibanaService } from '../../../state/kibana_service';

import {
PRIVATE_AVAILABLE_LABEL,
TEST_SCHEDULED_LABEL,
} from '../../overview/monitor_list/translations';

export interface ActionBarProps {
monitor: SyntheticsMonitor;
isValid: boolean;
Expand All @@ -63,9 +68,11 @@ export const ActionBar = ({
const [isSaving, setIsSaving] = useState(false);
const [isSuccessful, setIsSuccessful] = useState(false);
const [isPopoverOpen, setIsPopoverOpen] = useState<boolean | undefined>(undefined);
const mouseMoveTimeoutIds = useRef<[number, number]>([0, 0]);
const isReadOnly = monitor[ConfigKey.MONITOR_SOURCE_TYPE] === SourceType.PROJECT;

const hasServiceManagedLocation = monitor.locations?.some((loc) => loc.isServiceManaged);
const isOnlyPrivateLocations = !locations.some((loc) => loc.isServiceManaged);

const { data, status } = useFetcher(() => {
if (!isSaving || !isValid) {
Expand Down Expand Up @@ -139,11 +146,14 @@ export const ActionBar = ({
<EuiFlexItem grow={false}>
<WarningText>{!isValid && hasBeenSubmitted && VALIDATION_ERROR_LABEL}</WarningText>
</EuiFlexItem>

{onTestNow && (
<EuiFlexItem grow={false}>
{/* Popover is used instead of EuiTooltip until the resolution of https://github.com/elastic/eui/issues/5604 */}
<EuiPopover
repositionOnScroll={true}
ownFocus={false}
initialFocus={''}
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do when a value isn't specified?

Copy link
Contributor Author

@awahab07 awahab07 Aug 17, 2022

Choose a reason for hiding this comment

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

Here we don't want the popover to capture focus when it's opened, to behave like a tooltip. Providing an empty selector is the only option as undefined is the default value.

Without specifying the prop, it focuses the popover, stealing the focus from monitor form.

button={
<EuiButton
css={{ width: '100%' }}
Expand All @@ -154,11 +164,24 @@ export const ActionBar = ({
disabled={!isValid || isTestRunInProgress || !hasServiceManagedLocation}
data-test-subj={'monitorTestNowRunBtn'}
onClick={() => onTestNow()}
onMouseEnter={() => {
setIsPopoverOpen(true);
onMouseOver={() => {
// We need this custom logic to display a popover even when button is disabled.
clearTimeout(mouseMoveTimeoutIds.current[1]);
if (mouseMoveTimeoutIds.current[0] === 0) {
mouseMoveTimeoutIds.current[0] = setTimeout(() => {
clearTimeout(mouseMoveTimeoutIds.current[1]);
setIsPopoverOpen(true);
}, 250) as unknown as number;
}
}}
onMouseLeave={() => {
setIsPopoverOpen(false);
onMouseOut={() => {
// We need this custom logic to display a popover even when button is disabled.
clearTimeout(mouseMoveTimeoutIds.current[1]);
mouseMoveTimeoutIds.current[1] = setTimeout(() => {
clearTimeout(mouseMoveTimeoutIds.current[0]);
setIsPopoverOpen(false);
mouseMoveTimeoutIds.current = [0, 0];
}, 100) as unknown as number;
}}
>
{testRun ? RE_RUN_TEST_LABEL : RUN_TEST_LABEL}
Expand All @@ -167,7 +190,13 @@ export const ActionBar = ({
isOpen={isPopoverOpen}
>
<EuiText style={{ width: 260, outline: 'none' }}>
<p>{TEST_NOW_DESCRIPTION}</p>
<p>
{isTestRunInProgress
? TEST_SCHEDULED_LABEL
: isOnlyPrivateLocations || (isValid && !hasServiceManagedLocation)
? PRIVATE_AVAILABLE_LABEL
: TEST_NOW_DESCRIPTION}
</p>
</EuiText>
</EuiPopover>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ export function useRunOnceErrors({
}) {
const [locationErrors, setLocationErrors] = useState<ServiceLocationErrors>([]);
const [runOnceServiceError, setRunOnceServiceError] = useState<Error | null>(null);
const publicLocations = useMemo(
() => (locations ?? []).filter((loc) => loc.isServiceManaged),
[locations]
);

useEffect(() => {
setLocationErrors([]);
Expand All @@ -43,16 +47,16 @@ export function useRunOnceErrors({
}, [serviceError]);

const locationsById: Record<string, Locations[number]> = useMemo(
() => (locations as Locations).reduce((acc, cur) => ({ ...acc, [cur.id]: cur }), {}),
[locations]
() => (publicLocations as Locations).reduce((acc, cur) => ({ ...acc, [cur.id]: cur }), {}),
[publicLocations]
);

const expectPings =
locations.length - (locationErrors ?? []).filter(({ locationId }) => !!locationId).length;
publicLocations.length - (locationErrors ?? []).filter(({ locationId }) => !!locationId).length;

const hasBlockingError =
!!runOnceServiceError ||
(locationErrors?.length && locationErrors?.length === locations.length);
(locationErrors?.length && locationErrors?.length === publicLocations.length);

const errorMessages = useMemo(() => {
if (hasBlockingError) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
*/

import { EuiButtonIcon, EuiLoadingSpinner, EuiToolTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { Ping } from '../../../../../../common/runtime_types';
import { testNowMonitorAction } from '../../../../state/actions';
import { testNowRunSelector, TestRunStats } from '../../../../state/reducers/test_now_runs';
import * as labels from '../translations';

export const TestNowColumn = ({
monitorId,
Expand All @@ -28,15 +28,15 @@ export const TestNowColumn = ({

if (selectedMonitor.monitor.fleet_managed) {
return (
<EuiToolTip content={PRIVATE_AVAILABLE_LABEL}>
<EuiToolTip content={labels.PRIVATE_AVAILABLE_LABEL}>
<>--</>
</EuiToolTip>
);
}

if (!configId) {
return (
<EuiToolTip content={TEST_NOW_AVAILABLE_LABEL}>
<EuiToolTip content={labels.TEST_NOW_AVAILABLE_LABEL}>
<>--</>
</EuiToolTip>
);
Expand All @@ -54,45 +54,13 @@ export const TestNowColumn = ({
}

return (
<EuiToolTip content={testNowRun ? TEST_SCHEDULED_LABEL : TEST_NOW_LABEL}>
<EuiToolTip content={testNowRun ? labels.TEST_SCHEDULED_LABEL : labels.TEST_NOW_LABEL}>
<EuiButtonIcon
iconType="play"
onClick={() => testNowClick()}
isDisabled={!isTestNowCompleted}
aria-label={TEST_NOW_ARIA_LABEL}
aria-label={labels.TEST_NOW_ARIA_LABEL}
/>
</EuiToolTip>
);
};

export const TEST_NOW_ARIA_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.AriaLabel',
{
defaultMessage: 'CLick to run test now',
}
);

export const TEST_NOW_AVAILABLE_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.available',
{
defaultMessage: 'Test now is only available for monitors added via Monitor Management.',
}
);

export const PRIVATE_AVAILABLE_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.available.private',
{
defaultMessage: `You can't currently test monitors running on private locations on demand.`,
}
);

export const TEST_NOW_LABEL = i18n.translate('xpack.synthetics.monitorList.testNow.label', {
defaultMessage: 'Test now',
});

export const TEST_SCHEDULED_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.scheduled',
{
defaultMessage: 'Test is already scheduled',
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,35 @@ export const STATUS_ALERT_COLUMN = i18n.translate(
export const TEST_NOW_COLUMN = i18n.translate('xpack.synthetics.monitorList.testNow.label', {
defaultMessage: 'Test now',
});

export const TEST_NOW_AVAILABLE_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.available',
{
defaultMessage: 'Test now is only available for monitors added via Monitor Management.',
}
);

export const TEST_SCHEDULED_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.scheduled',
{
defaultMessage: 'Test is already scheduled',
}
);

export const PRIVATE_AVAILABLE_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.available.private',
{
defaultMessage: `You can't currently test monitors running on private locations on demand.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: `You can't currently test monitors running on private locations on demand.`,
defaultMessage: `Test now is currently not available for monitors running on private locations`,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The copy comes from here #137912 (comment).

}
);

export const TEST_NOW_ARIA_LABEL = i18n.translate(
'xpack.synthetics.monitorList.testNow.AriaLabel',
{
defaultMessage: 'Click to run test now',
}
);

export const TEST_NOW_LABEL = i18n.translate('xpack.synthetics.monitorList.testNow.label', {
defaultMessage: 'Test now',
});