Skip to content

Commit

Permalink
fix(actions): Better type checks for icons (opensearch-project#1496)
Browse files Browse the repository at this point in the history
- fix typo in iconType for `replace_panel` action
- change iconType types from `string` to `EuiIconType`
- use `icon` instead of `iconType` for svg pathseslin
- remove unnecessary icon assertions

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

Co-authored-by: Tommy Markley <markleyt@amazon.com>
  • Loading branch information
joshuarrrr and Tommy Markley committed Apr 28, 2022
1 parent 3f81862 commit 2a6a1ff
Show file tree
Hide file tree
Showing 26 changed files with 58 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@
* GitHub history for details.
*/

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 @@ -34,6 +34,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 @@ -192,7 +193,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 @@ -30,6 +30,7 @@
* GitHub history for details.
*/

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

Expand Down Expand Up @@ -77,7 +78,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 @@ -34,6 +34,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 @@ -54,7 +55,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 @@ -6,6 +6,8 @@
* compatible open source license.
*/

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 @@ -66,5 +68,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 @@ -33,6 +33,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 @@ -65,7 +66,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 @@ -34,6 +34,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 @@ -71,7 +72,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 @@ -30,6 +30,7 @@
* GitHub history for details.
*/

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 @@ -74,7 +75,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 @@ -33,6 +33,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 @@ -57,7 +58,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 @@ -78,7 +79,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 @@ -32,6 +32,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 @@ -68,11 +69,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 @@ -33,6 +33,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 @@ -65,7 +66,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 @@ -34,6 +34,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 @@ -89,7 +90,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 @@ -33,6 +33,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 @@ -61,7 +62,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 @@ -32,6 +32,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 @@ -58,7 +59,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 @@ -33,6 +33,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 @@ -54,7 +55,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 @@ -30,6 +30,7 @@
* GitHub history for details.
*/
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 @@ -64,7 +65,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 @@ -38,6 +38,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 @@ -91,7 +91,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 @@ -31,6 +31,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 @@ -91,6 +92,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`
}
4 changes: 3 additions & 1 deletion src/plugins/management/public/utils/management_item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

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 @@ -31,6 +31,7 @@
*/

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

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

Expand All @@ -44,7 +45,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 @@ -31,6 +31,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 @@ -87,7 +88,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 @@ -32,6 +32,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 @@ -55,7 +56,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 2a6a1ff

Please sign in to comment.