Skip to content

Commit

Permalink
fix(actions): Better type checks for icons (#1496)
Browse files Browse the repository at this point in the history
* fix(actions): Better type checks for icons

- fix typo in iconType for `replace_panel` action
- change iconType types from `string` to `EuiIconType`
- use `icon` instead of `iconType` for svg pathseslin

Signed-off-by: Josh Romero <rmerqg@amazon.com>

* fix(actions): Remove unnecessary icon assertions

Signed-off-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit ef3a9c0)
  • Loading branch information
joshuarrrr authored and github-actions[bot] committed Apr 29, 2022
1 parent e0a958b commit c6031a9
Show file tree
Hide file tree
Showing 26 changed files with 57 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@
* under the License.
*/

import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { Action } from '../../../../src/plugins/ui_actions/public';

export const sampleAction = (
id: string,
order: number,
name: string,
icon: string,
icon: EuiIconType,
grouping?: Action['grouping']
): Action => {
return {
Expand Down
3 changes: 2 additions & 1 deletion src/core/public/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { Observable } from 'rxjs';
import { History } from 'history';
import { RecursiveReadonly } from '@osd/utility-types';

import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { MountPoint } from '../types';
import { Capabilities } from './capabilities';
import { ChromeStart } from '../chrome';
Expand Down Expand Up @@ -190,7 +191,7 @@ export interface App<HistoryLocationState = unknown> {
* A EUI iconType that will be used for the app's icon. This icon
* takes precendence over the `icon` property.
*/
euiIconType?: string;
euiIconType?: EuiIconType;

/**
* A URL to an image file used as an icon. Used as a fallback
Expand Down
3 changes: 2 additions & 1 deletion src/core/public/chrome/nav_links/nav_link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* under the License.
*/

import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { pick } from '@osd/std';
import { AppCategory } from '../../';

Expand Down Expand Up @@ -75,7 +76,7 @@ export interface ChromeNavLink {
* A EUI iconType that will be used for the app's icon. This icon
* takes precedence over the `icon` property.
*/
readonly euiIconType?: string;
readonly euiIconType?: EuiIconType;

/**
* A URL to an image file used as an icon. Used as a fallback
Expand Down
3 changes: 2 additions & 1 deletion src/core/public/chrome/nav_links/to_nav_link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { PublicAppInfo, AppNavLinkStatus, AppStatus } from '../../application';
import { toNavLink } from './to_nav_link';

import { httpServiceMock } from '../../mocks';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';

const app = (props: Partial<PublicAppInfo> = {}): PublicAppInfo => ({
id: 'some-id',
Expand All @@ -52,7 +53,7 @@ describe('toNavLink', () => {
title: 'title',
order: 12,
tooltip: 'tooltip',
euiIconType: 'my-icon',
euiIconType: 'my-icon' as EuiIconType,
}),
basePath
);
Expand Down
4 changes: 3 additions & 1 deletion src/core/types/app_category.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
* GitHub history for details.
*/

import { EuiIconType } from '@elastic/eui/src/components/icon/icon';

/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
Expand Down Expand Up @@ -64,5 +66,5 @@ export interface AppCategory {
* If the category is only 1 item, and no icon is defined, will default to the product icon
* Defaults to initials if no icon is defined
*/
euiIconType?: string;
euiIconType?: EuiIconType;
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import { i18n } from '@osd/i18n';
import _ from 'lodash';
import uuid from 'uuid';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin';
import { ViewMode, PanelState, IEmbeddable } from '../../embeddable_plugin';
import {
Expand Down Expand Up @@ -63,7 +64,7 @@ export class AddToLibraryAction implements ActionByType<typeof ACTION_ADD_TO_LIB
});
}

public getIconType({ embeddable }: AddToLibraryActionContext) {
public getIconType({ embeddable }: AddToLibraryActionContext): EuiIconType {
if (!embeddable.getRoot() || !embeddable.getRoot().isContainer) {
throw new IncompatibleActionError();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { i18n } from '@osd/i18n';
import { CoreStart } from 'src/core/public';
import uuid from 'uuid';
import _ from 'lodash';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin';
import { ViewMode, PanelState, IEmbeddable } from '../../embeddable_plugin';
import { SavedObject } from '../../../../saved_objects/public';
Expand Down Expand Up @@ -69,7 +70,7 @@ export class ClonePanelAction implements ActionByType<typeof ACTION_CLONE_PANEL>
});
}

public getIconType({ embeddable }: ClonePanelActionContext) {
public getIconType({ embeddable }: ClonePanelActionContext): EuiIconType {
if (!embeddable.getRoot() || !embeddable.getRoot().isContainer) {
throw new IncompatibleActionError();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* under the License.
*/

import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { i18n } from '@osd/i18n';
import { IEmbeddable } from '../../embeddable_plugin';
import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin';
Expand Down Expand Up @@ -72,7 +73,7 @@ export class ExpandPanelAction implements ActionByType<typeof ACTION_EXPAND_PANE
});
}

public getIconType({ embeddable }: ExpandPanelActionContext) {
public getIconType({ embeddable }: ExpandPanelActionContext): EuiIconType {
if (!embeddable.parent || !isDashboard(embeddable.parent)) {
throw new IncompatibleActionError();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import React from 'react';
import { i18n } from '@osd/i18n';
import { EuiBadge } from '@elastic/eui';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import {
IEmbeddable,
ViewMode,
Expand All @@ -55,7 +56,7 @@ export class LibraryNotificationAction implements ActionByType<typeof ACTION_LIB
defaultMessage: 'Library',
});

private icon = 'folderCheck';
private icon = 'folderCheck' as const;

public readonly MenuItem = reactToUiComponent(() => (
<EuiBadge
Expand All @@ -76,7 +77,7 @@ export class LibraryNotificationAction implements ActionByType<typeof ACTION_LIB
return this.displayName;
}

public getIconType({ embeddable }: LibraryNotificationActionContext) {
public getIconType({ embeddable }: LibraryNotificationActionContext): EuiIconType {
if (!embeddable.getRoot() || !embeddable.getRoot().isContainer) {
throw new IncompatibleActionError();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import { i18n } from '@osd/i18n';
import { CoreStart } from 'src/core/public';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { IEmbeddable, ViewMode, EmbeddableStart } from '../../embeddable_plugin';
import { DASHBOARD_CONTAINER_TYPE, DashboardContainer } from '../embeddable';
import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin';
Expand Down Expand Up @@ -66,11 +67,11 @@ export class ReplacePanelAction implements ActionByType<typeof ACTION_REPLACE_PA
});
}

public getIconType({ embeddable }: ReplacePanelActionContext) {
public getIconType({ embeddable }: ReplacePanelActionContext): EuiIconType {
if (!embeddable.parent || !isDashboard(embeddable.parent)) {
throw new IncompatibleActionError();
}
return 'dqlOperand';
return 'kqlOperand';
}

public async isCompatible({ embeddable }: ReplacePanelActionContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import { i18n } from '@osd/i18n';
import _ from 'lodash';
import uuid from 'uuid';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin';
import { ViewMode, PanelState, IEmbeddable } from '../../embeddable_plugin';
import {
Expand Down Expand Up @@ -63,7 +64,7 @@ export class UnlinkFromLibraryAction implements ActionByType<typeof ACTION_UNLIN
});
}

public getIconType({ embeddable }: UnlinkFromLibraryActionContext) {
public getIconType({ embeddable }: UnlinkFromLibraryActionContext): EuiIconType {
if (!embeddable.getRoot() || !embeddable.getRoot().isContainer) {
throw new IncompatibleActionError();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { i18n } from '@osd/i18n';
import { ApplicationStart } from 'opensearch-dashboards/public';
import { Action } from 'src/plugins/ui_actions/public';
import { take } from 'rxjs/operators';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ViewMode } from '../types';
import { EmbeddableFactoryNotFoundError } from '../errors';
import { EmbeddableStart } from '../../plugin';
Expand Down Expand Up @@ -87,7 +88,7 @@ export class EditPanelAction implements Action<ActionContext> {
});
}

getIconType() {
getIconType(): EuiIconType {
return 'pencil';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { i18n } from '@osd/i18n';
import { Action, ActionExecutionContext } from 'src/plugins/ui_actions/public';
import { NotificationsStart, OverlayStart } from 'src/core/public';
import { EmbeddableStart } from 'src/plugins/embeddable/public/plugin';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ViewMode } from '../../../../types';
import { openAddPanelFlyout } from './open_add_panel_flyout';
import { IContainer } from '../../../../containers';
Expand Down Expand Up @@ -60,7 +61,7 @@ export class AddPanelAction implements Action<ActionContext> {
});
}

public getIconType() {
public getIconType(): EuiIconType {
return 'plusInCircleFilled';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import { i18n } from '@osd/i18n';
import { Action } from 'src/plugins/ui_actions/public';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ViewMode } from '../../../../types';
import { IEmbeddable } from '../../../../embeddables';

Expand All @@ -56,7 +57,7 @@ export class CustomizePanelTitleAction implements Action<ActionContext> {
});
}

public getIconType() {
public getIconType(): EuiIconType {
return 'pencil';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import { i18n } from '@osd/i18n';
import { Action } from 'src/plugins/ui_actions/public';
import { Start as InspectorStartContract } from 'src/plugins/inspector/public';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { IEmbeddable } from '../../../embeddables';

export const ACTION_INSPECT_PANEL = 'openInspector';
Expand All @@ -52,7 +53,7 @@ export class InspectPanelAction implements Action<ActionContext> {
});
}

public getIconType() {
public getIconType(): EuiIconType {
return 'inspect';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import { i18n } from '@osd/i18n';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { Action, IncompatibleActionError } from '../../../ui_actions';
import { ContainerInput, IContainer } from '../../../containers';
import { ViewMode } from '../../../types';
Expand Down Expand Up @@ -63,7 +64,7 @@ export class RemovePanelAction implements Action<ActionContext> {
});
}

public getIconType() {
public getIconType(): EuiIconType {
return 'trash';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { i18n } from '@osd/i18n';
import { I18nProvider } from '@osd/i18n/react';
import { StartServicesAccessor } from 'src/core/public';

import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { OpenSearchDashboardsContextProvider } from '../../../opensearch_dashboards_react/public';
import { ManagementAppMountParams } from '../../../management/public';
import {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/management/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class ManagementPlugin implements Plugin<ManagementSetup, ManagementStart
defaultMessage: 'Stack Management',
}),
order: 9040,
euiIconType: '/plugins/home/assets/logos/opensearch_mark_default.svg',
icon: '/plugins/home/assets/logos/opensearch_mark_default.svg',
category: DEFAULT_APP_CATEGORIES.management,
updater$: this.appUpdater,
async mount(params: AppMountParameters) {
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/management/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import { ScopedHistory, Capabilities } from 'opensearch-dashboards/public';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ManagementSection, RegisterManagementSectionArgs } from './utils';
import { ChromeBreadcrumb } from '../../../core/public/';

Expand Down Expand Up @@ -89,6 +90,6 @@ export interface CreateManagementItemArgs {
title: string;
tip?: string;
order?: number;
euiIconType?: string; // takes precedence over `icon` property.
euiIconType?: EuiIconType; // takes precedence over `icon` property.
icon?: string; // URL to image file; fallback if no `euiIconType`
}
3 changes: 2 additions & 1 deletion src/plugins/management/public/utils/management_item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@
* under the License.
*/

import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { CreateManagementItemArgs } from '../types';

export class ManagementItem {
public readonly id: string = '';
public readonly title: string;
public readonly tip?: string;
public readonly order: number;
public readonly euiIconType?: string;
public readonly euiIconType?: EuiIconType;
public readonly icon?: string;

public enabled: boolean = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import { EuiButtonProps } from '@elastic/eui';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';

export type TopNavMenuAction = (anchorElement: HTMLElement) => void;

Expand All @@ -42,7 +43,7 @@ export interface TopNavMenuData {
disableButton?: boolean | (() => boolean);
tooltip?: string | (() => string | undefined);
emphasize?: boolean;
iconType?: string;
iconType?: EuiIconType;
iconSide?: EuiButtonProps['iconSide'];
}

Expand Down
3 changes: 2 additions & 1 deletion src/plugins/ui_actions/public/actions/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import { UiComponent } from 'src/plugins/opensearch_dashboards_utils/public';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { ActionType, ActionContextMapping, BaseContext } from '../types';
import { Presentable } from '../util/presentable';
import { Trigger } from '../triggers';
Expand Down Expand Up @@ -85,7 +86,7 @@ export interface Action<Context extends BaseContext = {}, T = ActionType>
/**
* Optional EUI icon type that can be displayed along with the title.
*/
getIconType(context: ActionExecutionContext<Context>): string | undefined;
getIconType(context: ActionExecutionContext<Context>): EuiIconType | undefined;

/**
* Returns a title to be displayed to the user.
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/ui_actions/public/actions/action_internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

// @ts-ignore
import React from 'react';
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
import { Action, ActionContext as Context, ActionDefinition } from './action';
import { Presentable, PresentableGrouping } from '../util/presentable';
import { uiToReactComponent } from '../../../opensearch_dashboards_react/public';
Expand All @@ -53,7 +54,7 @@ export class ActionInternal<A extends ActionDefinition = ActionDefinition>
return this.definition.execute(context);
}

public getIconType(context: Context<A>): string | undefined {
public getIconType(context: Context<A>): EuiIconType | undefined {
if (!this.definition.getIconType) return undefined;
return this.definition.getIconType(context);
}
Expand Down
Loading

0 comments on commit c6031a9

Please sign in to comment.