Skip to content

Commit

Permalink
[8.x] Improve URL drilldown authoring experience (#197454) (#198796)
Browse files Browse the repository at this point in the history
# Backport

This will backport the following commits from `main` to `8.x`:
- [Improve URL drilldown authoring experience
(#197454)](#197454)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Anton
Dosov","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-04T14:16:11Z","message":"Improve
URL drilldown authoring experience (#197454)\n\n## Summary\r\n\r\nclose
https://github.com/elastic/kibana/issues/163642\r\nclose
https://github.com/elastic/kibana/issues/163641\r\n\r\nAs part of
fix-it-week I picked up some of existing URL drilldown\r\nauthoring
issues hoping to improve it a bit with a low effort (we don't\r\nwant to
spend to much time on it).\r\n\r\nCurrent URL drilldown authoring
experience is terrible mainly because\r\nthere is no proper validation
while creating the drilldown as we don't\r\nhave the needed runtime
context. In the initial version we had a preview\r\nbut it was very
limited and used \"dummy\" context and in some cases got\r\nin the way
by blocking the \"save\" button for URLs that would have been\r\nvalid
in runtime. We simply removed the preview and validaiton on
some\r\npoint later, so you can create an URL drilldown only by trial
and error.\r\nThis is still the case in this PR, but it slightly improve
the\r\nexperience:\r\n\r\nFirstly, **ONLY IN EDIT MODE** instead of
hidding \"invalid\" drilldowns,\r\nwe're showing them now with an error.
This helps to find broken\r\ndrilldowns and address issues.
fixes\r\nhttps://github.com//issues/163641\r\n\r\n![Screenshot
2024-10-24 at 12
02\r\n25](https://github.com/user-attachments/assets/2e33ad91-2425-417d-b44f-faff74fccbab)\r\n\r\nThis
is far from ideal, but this is better from what we have now. \r\nAs for
the error UI I wanted to use EuiIconTip, but it doesn't work
well\r\nwhen inside that context menu because tooltip is shown below the
menu. I\r\ndidn't want to change zIndex as it might cause regressions in
other\r\nplaces, so I went for this inline error truncated after 3 lines
and the\r\nwhole error is shown in native browser tooltip when hovered.
The error\r\nis also printed in console\r\n\r\nIn addition to that I've
slightly improved the form editor experience \r\n- Show simple
non-blocking validation error (if URL is missing or the\r\npattern
doesn't look like an URL)\r\n- Add a help text about how to test and
properly validate \r\n\r\n<img width=\"576\" alt=\"Screenshot 2024-10-24
at 15 35
01\"\r\nsrc=\"https://github.com/user-attachments/assets/248b5c03-d445-4b30-97a2-a0a9d6a055a4\">\r\n<img
width=\"547\" alt=\"Screenshot 2024-10-24 at 15 35
08\"\r\nsrc=\"https://github.com/user-attachments/assets/c265ee1c-94ec-4238-85fb-fc90f069f3b4\">\r\n\r\n\r\nThis
is again, not ideal, but slighltly better then
before","sha":"014b956002fab12064e2ac729c09b1713ef8deac","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Drilldowns","v9.0.0","Team:SharedUX","backport:prev-minor"],"title":"Improve
URL drilldown authoring experience
","number":197454,"url":"https://github.com/elastic/kibana/pull/197454","mergeCommit":{"message":"Improve
URL drilldown authoring experience (#197454)\n\n## Summary\r\n\r\nclose
https://github.com/elastic/kibana/issues/163642\r\nclose
https://github.com/elastic/kibana/issues/163641\r\n\r\nAs part of
fix-it-week I picked up some of existing URL drilldown\r\nauthoring
issues hoping to improve it a bit with a low effort (we don't\r\nwant to
spend to much time on it).\r\n\r\nCurrent URL drilldown authoring
experience is terrible mainly because\r\nthere is no proper validation
while creating the drilldown as we don't\r\nhave the needed runtime
context. In the initial version we had a preview\r\nbut it was very
limited and used \"dummy\" context and in some cases got\r\nin the way
by blocking the \"save\" button for URLs that would have been\r\nvalid
in runtime. We simply removed the preview and validaiton on
some\r\npoint later, so you can create an URL drilldown only by trial
and error.\r\nThis is still the case in this PR, but it slightly improve
the\r\nexperience:\r\n\r\nFirstly, **ONLY IN EDIT MODE** instead of
hidding \"invalid\" drilldowns,\r\nwe're showing them now with an error.
This helps to find broken\r\ndrilldowns and address issues.
fixes\r\nhttps://github.com//issues/163641\r\n\r\n![Screenshot
2024-10-24 at 12
02\r\n25](https://github.com/user-attachments/assets/2e33ad91-2425-417d-b44f-faff74fccbab)\r\n\r\nThis
is far from ideal, but this is better from what we have now. \r\nAs for
the error UI I wanted to use EuiIconTip, but it doesn't work
well\r\nwhen inside that context menu because tooltip is shown below the
menu. I\r\ndidn't want to change zIndex as it might cause regressions in
other\r\nplaces, so I went for this inline error truncated after 3 lines
and the\r\nwhole error is shown in native browser tooltip when hovered.
The error\r\nis also printed in console\r\n\r\nIn addition to that I've
slightly improved the form editor experience \r\n- Show simple
non-blocking validation error (if URL is missing or the\r\npattern
doesn't look like an URL)\r\n- Add a help text about how to test and
properly validate \r\n\r\n<img width=\"576\" alt=\"Screenshot 2024-10-24
at 15 35
01\"\r\nsrc=\"https://github.com/user-attachments/assets/248b5c03-d445-4b30-97a2-a0a9d6a055a4\">\r\n<img
width=\"547\" alt=\"Screenshot 2024-10-24 at 15 35
08\"\r\nsrc=\"https://github.com/user-attachments/assets/c265ee1c-94ec-4238-85fb-fc90f069f3b4\">\r\n\r\n\r\nThis
is again, not ideal, but slighltly better then
before","sha":"014b956002fab12064e2ac729c09b1713ef8deac"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197454","number":197454,"mergeCommit":{"message":"Improve
URL drilldown authoring experience (#197454)\n\n## Summary\r\n\r\nclose
https://github.com/elastic/kibana/issues/163642\r\nclose
https://github.com/elastic/kibana/issues/163641\r\n\r\nAs part of
fix-it-week I picked up some of existing URL drilldown\r\nauthoring
issues hoping to improve it a bit with a low effort (we don't\r\nwant to
spend to much time on it).\r\n\r\nCurrent URL drilldown authoring
experience is terrible mainly because\r\nthere is no proper validation
while creating the drilldown as we don't\r\nhave the needed runtime
context. In the initial version we had a preview\r\nbut it was very
limited and used \"dummy\" context and in some cases got\r\nin the way
by blocking the \"save\" button for URLs that would have been\r\nvalid
in runtime. We simply removed the preview and validaiton on
some\r\npoint later, so you can create an URL drilldown only by trial
and error.\r\nThis is still the case in this PR, but it slightly improve
the\r\nexperience:\r\n\r\nFirstly, **ONLY IN EDIT MODE** instead of
hidding \"invalid\" drilldowns,\r\nwe're showing them now with an error.
This helps to find broken\r\ndrilldowns and address issues.
fixes\r\nhttps://github.com//issues/163641\r\n\r\n![Screenshot
2024-10-24 at 12
02\r\n25](https://github.com/user-attachments/assets/2e33ad91-2425-417d-b44f-faff74fccbab)\r\n\r\nThis
is far from ideal, but this is better from what we have now. \r\nAs for
the error UI I wanted to use EuiIconTip, but it doesn't work
well\r\nwhen inside that context menu because tooltip is shown below the
menu. I\r\ndidn't want to change zIndex as it might cause regressions in
other\r\nplaces, so I went for this inline error truncated after 3 lines
and the\r\nwhole error is shown in native browser tooltip when hovered.
The error\r\nis also printed in console\r\n\r\nIn addition to that I've
slightly improved the form editor experience \r\n- Show simple
non-blocking validation error (if URL is missing or the\r\npattern
doesn't look like an URL)\r\n- Add a help text about how to test and
properly validate \r\n\r\n<img width=\"576\" alt=\"Screenshot 2024-10-24
at 15 35
01\"\r\nsrc=\"https://github.com/user-attachments/assets/248b5c03-d445-4b30-97a2-a0a9d6a055a4\">\r\n<img
width=\"547\" alt=\"Screenshot 2024-10-24 at 15 35
08\"\r\nsrc=\"https://github.com/user-attachments/assets/c265ee1c-94ec-4238-85fb-fc90f069f3b4\">\r\n\r\n\r\nThis
is again, not ideal, but slighltly better then
before","sha":"014b956002fab12064e2ac729c09b1713ef8deac"}}]}]
BACKPORT-->

Co-authored-by: Anton Dosov <[email protected]>
  • Loading branch information
kibanamachine and Dosant authored Nov 4, 2024
1 parent 2f0a68a commit 88926f3
Show file tree
Hide file tree
Showing 12 changed files with 291 additions and 126 deletions.
5 changes: 0 additions & 5 deletions src/plugins/ui_actions_enhanced/.eslintrc.json

This file was deleted.

3 changes: 3 additions & 0 deletions src/plugins/ui_actions_enhanced/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { SerializableRecord } from '@kbn/utility-types';

export type BaseActionConfig = SerializableRecord;

// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
export type SerializedAction<Config extends BaseActionConfig = BaseActionConfig> = {
readonly factoryId: string;
readonly name: string;
Expand All @@ -20,12 +21,14 @@ export type SerializedAction<Config extends BaseActionConfig = BaseActionConfig>
/**
* Serialized representation of a triggers-action pair, used to persist in storage.
*/
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
export type SerializedEvent = {
eventId: string;
triggers: string[];
action: SerializedAction;
};

// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
export type DynamicActionsState = {
events: SerializedEvent[];
};
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const dashboards = [
{ id: 'dashboard2', title: 'Dashboard 2' },
];

// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
type DashboardDrilldownConfig = {
dashboardId?: string;
useCurrentFilters: boolean;
Expand Down Expand Up @@ -119,6 +120,7 @@ export const dashboardFactory = new ActionFactory(dashboardDrilldownActionFactor
getFeatureUsageStart: () => licensingMock.createStart().featureUsage,
});

// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
type UrlDrilldownConfig = {
url: string;
openInNewTab: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,48 +9,50 @@

import { i18n } from '@kbn/i18n';

export const txtUrlTemplatePlaceholder = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplatePlaceholderText',
{
defaultMessage: 'Example: {exampleUrl}',
values: {
exampleUrl: 'https://www.my-url.com/?{{event.key}}={{event.value}}',
},
}
);

export const txtUrlPreviewHelpText = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlPreviewHelpText',
{
defaultMessage: `Please note that in preview '{{event.*}}' variables are substituted with dummy values.`,
}
);

export const txtUrlTemplateLabel = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplateLabel',
{
defaultMessage: 'Enter URL',
}
);

export const txtUrlTemplateSyntaxHelpLinkText = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplateSyntaxHelpLinkText',
export const txtEmptyErrorMessage = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplateEmptyErrorMessage',
{
defaultMessage: 'Syntax help',
defaultMessage: 'URL template is required.',
}
);

export const txtUrlTemplatePreviewLabel = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlPreviewLabel',
export const txtInvalidFormatErrorMessage = ({
error,
example,
}: {
error: string;
example: string;
}) =>
i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplateInvalidFormatErrorMessage',
{
defaultMessage: '{error} Example: {example}',
values: {
error,
example,
},
}
);

export const txtUrlTemplateSyntaxTestingHelpText = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplateSyntaxTestingHelpText',
{
defaultMessage: 'URL preview:',
defaultMessage:
'To validate and test the URL template, save the configuration and use this drilldown from the panel.',
}
);

export const txtUrlTemplatePreviewLinkText = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlPreviewLinkText',
export const txtUrlTemplateSyntaxHelpLinkText = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.urlTemplateSyntaxHelpLinkText',
{
defaultMessage: 'Preview',
defaultMessage: 'Syntax help',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ import {
txtUrlTemplateSyntaxHelpLinkText,
txtUrlTemplateLabel,
txtUrlTemplateAdditionalOptions,
txtEmptyErrorMessage,
txtInvalidFormatErrorMessage,
txtUrlTemplateSyntaxTestingHelpText,
} from './i18n';
import { VariablePopover } from '../variable_popover';
import { UrlDrilldownOptionsComponent } from './lazy';
import { DEFAULT_URL_DRILLDOWN_OPTIONS } from '../../constants';
import { validateUrl } from '../../url_validation';

export interface UrlDrilldownCollectConfigProps {
config: UrlDrilldownConfig;
Expand Down Expand Up @@ -69,7 +73,16 @@ export const UrlDrilldownCollectConfig: React.FC<UrlDrilldownCollectConfigProps>
}
}
const isEmpty = !urlTemplate;
const isInvalid = !isPristine && isEmpty;

const isValidUrlFormat = validateUrl(urlTemplate);
const isInvalid = !isPristine && (isEmpty || !isValidUrlFormat.isValid);

const invalidErrorMessage = isInvalid
? isEmpty
? txtEmptyErrorMessage
: txtInvalidFormatErrorMessage({ error: isValidUrlFormat.error!, example: exampleUrl })
: undefined;

const variablesDropdown = (
<VariablePopover
variables={variables}
Expand All @@ -91,14 +104,18 @@ export const UrlDrilldownCollectConfig: React.FC<UrlDrilldownCollectConfigProps>
<EuiFormRow
fullWidth
isInvalid={isInvalid}
error={invalidErrorMessage}
className={'uaeUrlDrilldownCollectConfig__urlTemplateFormRow'}
label={txtUrlTemplateLabel}
helpText={
syntaxHelpDocsLink && (
<EuiLink external target={'_blank'} href={syntaxHelpDocsLink}>
{txtUrlTemplateSyntaxHelpLinkText}
</EuiLink>
)
<>
{txtUrlTemplateSyntaxTestingHelpText}{' '}
{syntaxHelpDocsLink ? (
<EuiLink external target={'_blank'} href={syntaxHelpDocsLink}>
{txtUrlTemplateSyntaxHelpLinkText}
</EuiLink>
) : null}
</>
}
labelAppend={variablesDropdown}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type UrlDrilldownConfig = {
/**
* User-configurable options for URL drilldowns
*/
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
export type UrlDrilldownOptions = {
openInNewTab: boolean;
encodeUrl: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,24 @@ import { compile } from './url_template';
const generalFormatError = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownValidation.urlFormatGeneralErrorMessage',
{
defaultMessage: 'Invalid format. Example: {exampleUrl}',
values: {
exampleUrl: 'https://www.my-url.com/?{{event.key}}={{event.value}}',
},
defaultMessage: 'Invalid URL format.',
}
);

const formatError = (message: string) =>
i18n.translate('uiActionsEnhanced.drilldowns.urlDrilldownValidation.urlFormatErrorMessage', {
defaultMessage: 'Invalid format: {message}',
const compileError = (message: string) =>
i18n.translate('uiActionsEnhanced.drilldowns.urlDrilldownValidation.urlCompileErrorMessage', {
defaultMessage: 'The URL template is not valid in the given context. {message}.',
values: {
message,
message: message.replaceAll('[object Object]', 'context'),
},
});

const SAFE_URL_PATTERN = /^(?:(?:https?|mailto):|[^&:/?#]*(?:[/?#]|$))/gi;
export function validateUrl(url: string): { isValid: boolean; error?: string } {
export function validateUrl(url: string): {
isValid: boolean;
error?: string;
invalidUrl?: string;
} {
if (!url)
return {
isValid: false,
Expand All @@ -45,27 +46,40 @@ export function validateUrl(url: string): { isValid: boolean; error?: string } {
return {
isValid: false,
error: generalFormatError,
invalidUrl: url,
};
}
}

export async function validateUrlTemplate(
urlTemplate: UrlDrilldownConfig['url'],
scope: UrlDrilldownScope
): Promise<{ isValid: boolean; error?: string }> {
): Promise<{ isValid: boolean; error?: string; invalidUrl?: string }> {
if (!urlTemplate.template)
return {
isValid: false,
error: generalFormatError,
};

let compiledUrl: string;

try {
compiledUrl = await compile(urlTemplate.template, scope);
} catch (e) {
return {
isValid: false,
error: compileError(e.message),
invalidUrl: urlTemplate.template,
};
}

try {
const compiledUrl = await compile(urlTemplate.template, scope);
return validateUrl(compiledUrl);
} catch (e) {
return {
isValid: false,
error: formatError(e.message),
error: generalFormatError + ` ${e.message}.`,
invalidUrl: compiledUrl,
};
}
}
Loading

0 comments on commit 88926f3

Please sign in to comment.