From 836b9b4bc67641f4ca5e6befbc21cb0a65b6e528 Mon Sep 17 00:00:00 2001 From: Stephan Lee Date: Thu, 26 Aug 2021 22:24:19 -0700 Subject: [PATCH] timeseries: show empty match warning text When tag filter AND visualization filter matches no cards, TimeSeries should some some message. This change adds the warning when user inputed entries that would match no cards at all. --- tensorboard/webapp/metrics/views/_common.scss | 8 ++ .../webapp/metrics/views/main_view/BUILD | 18 +++++ .../views/main_view/card_groups_container.ts | 25 +------ .../views/main_view/common_selectors.ts | 51 +++++++++++++ .../empty_tag_match_message_component.ts | 74 +++++++++++++++++++ .../empty_tag_match_message_container.ts | 55 ++++++++++++++ .../main_view/filtered_view_component.scss | 5 ++ .../main_view/filtered_view_component.ts | 5 ++ .../main_view/filtered_view_container.ts | 51 ++++--------- .../views/main_view/main_view_module.ts | 4 + .../metrics/views/main_view/main_view_test.ts | 42 +++++++++++ .../main_view/pinned_view_component.scss | 6 +- 12 files changed, 283 insertions(+), 61 deletions(-) create mode 100644 tensorboard/webapp/metrics/views/main_view/common_selectors.ts create mode 100644 tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_component.ts create mode 100644 tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_container.ts diff --git a/tensorboard/webapp/metrics/views/_common.scss b/tensorboard/webapp/metrics/views/_common.scss index 2c389f1ed2..9416052bf7 100644 --- a/tensorboard/webapp/metrics/views/_common.scss +++ b/tensorboard/webapp/metrics/views/_common.scss @@ -62,3 +62,11 @@ $metrics-min-card-height: 320px; @include tb-theme-foreground-prop(color, secondary-text); white-space: nowrap; } + +@mixin metrics-empty-message { + @include tb-theme-foreground-prop(color, secondary-text); + font-size: 13px; + font-style: italic; + padding: 16px; + text-align: center; +} diff --git a/tensorboard/webapp/metrics/views/main_view/BUILD b/tensorboard/webapp/metrics/views/main_view/BUILD index 530ca0e49e..fd539b98a8 100644 --- a/tensorboard/webapp/metrics/views/main_view/BUILD +++ b/tensorboard/webapp/metrics/views/main_view/BUILD @@ -47,6 +47,21 @@ tf_sass_binary( ], ) +tf_ts_library( + name = "common_selectors", + srcs = ["common_selectors.ts"], + deps = [ + "//tensorboard/webapp:app_state", + "//tensorboard/webapp:selectors", + "//tensorboard/webapp/metrics/data_source", + "//tensorboard/webapp/metrics/store", + "//tensorboard/webapp/metrics/views:types", + "//tensorboard/webapp/metrics/views:utils", + "//tensorboard/webapp/util:types", + "@npm//@ngrx/store", + ], +) + tf_ng_module( name = "main_view", srcs = [ @@ -54,6 +69,8 @@ tf_ng_module( "card_grid_container.ts", "card_groups_component.ts", "card_groups_container.ts", + "empty_tag_match_message_component.ts", + "empty_tag_match_message_container.ts", "filter_input_component.ts", "filter_input_container.ts", "filtered_view_component.ts", @@ -76,6 +93,7 @@ tf_ng_module( ":pinned_view_component_styles", ], deps = [ + ":common_selectors", "//tensorboard/webapp:app_state", "//tensorboard/webapp:selectors", "//tensorboard/webapp/angular:expect_angular_cdk_scrolling", diff --git a/tensorboard/webapp/metrics/views/main_view/card_groups_container.ts b/tensorboard/webapp/metrics/views/main_view/card_groups_container.ts index c3f3ea15fa..79b701fce4 100644 --- a/tensorboard/webapp/metrics/views/main_view/card_groups_container.ts +++ b/tensorboard/webapp/metrics/views/main_view/card_groups_container.ts @@ -13,33 +13,16 @@ See the License for the specific language governing permissions and limitations under the License. ==============================================================================*/ import {ChangeDetectionStrategy, Component, Input} from '@angular/core'; -import {createSelector, Store} from '@ngrx/store'; +import {Store} from '@ngrx/store'; import {Observable} from 'rxjs'; import {combineLatestWith, map} from 'rxjs/operators'; import {State} from '../../../app_state'; -import {getCurrentRouteRunSelection} from '../../../selectors'; -import {isSingleRunPlugin} from '../../data_source'; -import { - getMetricsFilteredPluginTypes, - getNonEmptyCardIdsWithMetadata, -} from '../../store'; +import {getMetricsFilteredPluginTypes} from '../../store'; import {CardObserver} from '../card_renderer/card_lazy_loader'; import {CardGroup} from '../metrics_view_types'; import {groupCardIdWithMetdata} from '../utils'; - -const getRenderableCardIdsWithMetadata = createSelector( - getNonEmptyCardIdsWithMetadata, - getCurrentRouteRunSelection, - (cardList, runSelectionMap) => { - return cardList.filter((card) => { - if (!isSingleRunPlugin(card.plugin)) { - return true; - } - return Boolean(runSelectionMap && runSelectionMap.get(card.runId!)); - }); - } -); +import {getSortedRenderableCardIdsWithMetadata} from './common_selectors'; @Component({ selector: 'metrics-card-groups', @@ -57,7 +40,7 @@ export class CardGroupsContainer { constructor(private readonly store: Store) {} readonly cardGroups$: Observable = this.store - .select(getRenderableCardIdsWithMetadata) + .select(getSortedRenderableCardIdsWithMetadata) .pipe( combineLatestWith(this.store.select(getMetricsFilteredPluginTypes)), map(([cardList, filteredPlugins]) => { diff --git a/tensorboard/webapp/metrics/views/main_view/common_selectors.ts b/tensorboard/webapp/metrics/views/main_view/common_selectors.ts new file mode 100644 index 0000000000..ba80127f48 --- /dev/null +++ b/tensorboard/webapp/metrics/views/main_view/common_selectors.ts @@ -0,0 +1,51 @@ +/* Copyright 2021 The TensorFlow Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +==============================================================================*/ +import {createSelector} from '@ngrx/store'; + +import {State} from '../../../app_state'; +import {getCurrentRouteRunSelection} from '../../../selectors'; +import {DeepReadonly} from '../../../util/types'; +import {isSingleRunPlugin} from '../../data_source'; +import {getNonEmptyCardIdsWithMetadata} from '../../store'; +import {CardIdWithMetadata} from '../metrics_view_types'; +import {compareTagNames} from '../utils'; + +const getRenderableCardIdsWithMetadata = createSelector< + State, + readonly DeepReadonly[], + Map | null, + DeepReadonly[] +>( + getNonEmptyCardIdsWithMetadata, + getCurrentRouteRunSelection, + (cardList, runSelectionMap) => { + return cardList.filter((card) => { + if (!isSingleRunPlugin(card.plugin)) { + return true; + } + return Boolean(runSelectionMap && runSelectionMap.get(card.runId!)); + }); + } +); + +export const getSortedRenderableCardIdsWithMetadata = createSelector< + State, + DeepReadonly[], + DeepReadonly[] +>(getRenderableCardIdsWithMetadata, (cardList) => { + return cardList.sort((cardA, cardB) => { + return compareTagNames(cardA.tag, cardB.tag); + }); +}); diff --git a/tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_component.ts b/tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_component.ts new file mode 100644 index 0000000000..a9682636bb --- /dev/null +++ b/tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_component.ts @@ -0,0 +1,74 @@ +/* Copyright 2021 The TensorFlow Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +==============================================================================*/ +import {ChangeDetectionStrategy, Component, Input} from '@angular/core'; + +import {PluginType} from '../../data_source'; + +declare namespace Intl { + class ListFormat { + constructor( + locale?: string, + options?: { + localeMatcher?: 'lookup' | 'best fit'; + style?: 'long' | 'short' | 'narrow'; + type?: 'unit' | 'conjunction' | 'disjunction'; + } + ); + format: (items: string[]) => string; + } +} + +@Component({ + selector: 'metrics-empty-tag-match-component', + template: `No cards matches tag filter + /{{ tagFilterRegex }}/ among {{ tagCounts | number }} tags + and {{ getPluginTypeFilterString(pluginTypes) }} visualization + filter.`, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class EmptyTagMatchMessageComponent { + readonly PluginType = PluginType; + private readonly listFormatter = new Intl.ListFormat(undefined, { + style: 'long', + type: 'disjunction', + }); + + @Input() pluginTypes!: Set; + @Input() tagFilterRegex!: string; + @Input() tagCounts!: number; + + getPluginTypeFilterString(pluginTypes: Set): string { + const humanReadableTypes = [...pluginTypes].map((type) => { + switch (type) { + case PluginType.SCALARS: + return 'scalar'; + case PluginType.IMAGES: + return 'image'; + case PluginType.HISTOGRAMS: + return 'histogram'; + default: + const _: never = type; + throw new RangeError( + `Please implement human readable name for plugin type: ${type}` + ); + } + }); + + return this.listFormatter.format(humanReadableTypes); + } +} diff --git a/tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_container.ts b/tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_container.ts new file mode 100644 index 0000000000..5bf0a8c641 --- /dev/null +++ b/tensorboard/webapp/metrics/views/main_view/empty_tag_match_message_container.ts @@ -0,0 +1,55 @@ +/* Copyright 2021 The TensorFlow Authors. All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +==============================================================================*/ +import {ChangeDetectionStrategy, Component} from '@angular/core'; +import {Store} from '@ngrx/store'; +import {Observable} from 'rxjs'; +import {map} from 'rxjs/operators'; + +import {State} from '../../../app_state'; +import {PluginType} from '../../data_source'; +import {getMetricsFilteredPluginTypes, getMetricsTagFilter} from '../../store'; +import {getSortedRenderableCardIdsWithMetadata} from './common_selectors'; + +/** + * Warning message that displays when no tags do not match filter query. + */ +@Component({ + selector: 'metrics-empty-tag-match', + template: ` + + `, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class EmptyTagMatchMessageContainer { + constructor(private readonly store: Store) {} + + readonly pluginTypes$: Observable> = this.store.select( + getMetricsFilteredPluginTypes + ); + readonly tagFilterRegex$: Observable = this.store.select( + getMetricsTagFilter + ); + readonly tagCounts$: Observable = this.store + .select(getSortedRenderableCardIdsWithMetadata) + .pipe( + map((cardList) => { + return new Set(cardList.map(({tag}) => tag)).size; + }) + ); +} diff --git a/tensorboard/webapp/metrics/views/main_view/filtered_view_component.scss b/tensorboard/webapp/metrics/views/main_view/filtered_view_component.scss index a5444a821e..e34c74407a 100644 --- a/tensorboard/webapp/metrics/views/main_view/filtered_view_component.scss +++ b/tensorboard/webapp/metrics/views/main_view/filtered_view_component.scss @@ -32,3 +32,8 @@ limitations under the License. @include metrics-card-group-count-text; margin-left: 6px; } + +metrics-empty-tag-match { + @include metrics-empty-message; + display: block; +} diff --git a/tensorboard/webapp/metrics/views/main_view/filtered_view_component.ts b/tensorboard/webapp/metrics/views/main_view/filtered_view_component.ts index 52034bd44d..0605ada186 100644 --- a/tensorboard/webapp/metrics/views/main_view/filtered_view_component.ts +++ b/tensorboard/webapp/metrics/views/main_view/filtered_view_component.ts @@ -30,6 +30,10 @@ import {CardIdWithMetadata} from '../metrics_view_types'; > + { - return cardList.filter((card) => { - if (!isSingleRunPlugin(card.plugin)) { - return true; - } - return Boolean(runSelectionMap && runSelectionMap.get(card.runId!)); - }); - } -); - /** * An area showing cards that match the tag filter. */ @@ -62,6 +41,7 @@ const getRenderableCardIdsWithMetadata = createSelector( selector: 'metrics-filtered-view', template: ` @@ -75,16 +55,7 @@ export class FilteredViewContainer { readonly cardIdsWithMetadata$: Observable< DeepReadonly[] - > = this.store.select(getRenderableCardIdsWithMetadata).pipe( - // Pre-sort the tags since the list of tags do not change w.r.t the - // tagFilter regex. Since regexFilter can change often, this would allow - // us to save time from sorting thousands of tags at every keystroke which - // actually makes notably UI slower. - map((cardList) => { - return cardList.sort((cardA, cardB) => { - return compareTagNames(cardA.tag, cardB.tag); - }); - }), + > = this.store.select(getSortedRenderableCardIdsWithMetadata).pipe( combineLatestWith(this.store.select(getMetricsFilteredPluginTypes)), map(([cardList, filteredPluginTypes]) => { if (!filteredPluginTypes.size) return cardList; @@ -113,6 +84,16 @@ export class FilteredViewContainer { }); } ), + share(), startWith([]) ) as Observable[]>; + + readonly isEmptyMatch$: Observable = this.cardIdsWithMetadata$.pipe( + combineLatestWith( + this.store.select(getSortedRenderableCardIdsWithMetadata) + ), + map(([filteredCardList, fullCardList]) => { + return Boolean(fullCardList.length) && filteredCardList.length === 0; + }) + ); } diff --git a/tensorboard/webapp/metrics/views/main_view/main_view_module.ts b/tensorboard/webapp/metrics/views/main_view/main_view_module.ts index abcaab474c..0a669a9b03 100644 --- a/tensorboard/webapp/metrics/views/main_view/main_view_module.ts +++ b/tensorboard/webapp/metrics/views/main_view/main_view_module.ts @@ -29,6 +29,8 @@ import {CardGridComponent} from './card_grid_component'; import {CardGridContainer} from './card_grid_container'; import {CardGroupsComponent} from './card_groups_component'; import {CardGroupsContainer} from './card_groups_container'; +import {EmptyTagMatchMessageComponent} from './empty_tag_match_message_component'; +import {EmptyTagMatchMessageContainer} from './empty_tag_match_message_container'; import {FilteredViewComponent} from './filtered_view_component'; import {FilteredViewContainer} from './filtered_view_container'; import {MetricsFilterInputComponent} from './filter_input_component'; @@ -44,6 +46,8 @@ import {PinnedViewContainer} from './pinned_view_container'; CardGridContainer, CardGroupsComponent, CardGroupsContainer, + EmptyTagMatchMessageComponent, + EmptyTagMatchMessageContainer, FilteredViewComponent, FilteredViewContainer, MainViewComponent, diff --git a/tensorboard/webapp/metrics/views/main_view/main_view_test.ts b/tensorboard/webapp/metrics/views/main_view/main_view_test.ts index 21ecda5e87..12fb27049c 100644 --- a/tensorboard/webapp/metrics/views/main_view/main_view_test.ts +++ b/tensorboard/webapp/metrics/views/main_view/main_view_test.ts @@ -51,6 +51,8 @@ import {CardGridComponent} from './card_grid_component'; import {CardGridContainer} from './card_grid_container'; import {CardGroupsComponent} from './card_groups_component'; import {CardGroupsContainer} from './card_groups_container'; +import {EmptyTagMatchMessageComponent} from './empty_tag_match_message_component'; +import {EmptyTagMatchMessageContainer} from './empty_tag_match_message_container'; import {FilteredViewComponent} from './filtered_view_component'; import { FilteredViewContainer, @@ -153,6 +155,8 @@ describe('metrics main view', () => { CardGridContainer, CardGroupsComponent, CardGroupsContainer, + EmptyTagMatchMessageComponent, + EmptyTagMatchMessageContainer, FilteredViewComponent, FilteredViewContainer, MainViewComponent, @@ -1236,6 +1240,44 @@ describe('metrics main view', () => { expect(getFilterviewCardContents(fixture)).toEqual([]); })); + it('shows a warning when no cards match current query', fakeAsync(() => { + store.overrideSelector( + selectors.getNonEmptyCardIdsWithMetadata, + createNScalarCards(100) + ); + const fixture = createComponent('^no_match_please$'); + + expect( + fixture.debugElement + .query(By.css('metrics-filtered-view')) + .nativeElement.textContent.trim() + ).toContain( + 'No cards matches tag filter /^no_match_please$/ among 100 tags.' + ); + })); + + it('shows a warning about unmatched plugin type', fakeAsync(() => { + store.overrideSelector( + selectors.getMetricsFilteredPluginTypes, + new Set([PluginType.IMAGES, PluginType.HISTOGRAMS]) + ); + store.overrideSelector( + selectors.getNonEmptyCardIdsWithMetadata, + createNScalarCards(100) + ); + + const fixture = createComponent('.'); + + expect( + fixture.debugElement + .query(By.css('metrics-filtered-view')) + .nativeElement.textContent.trim() + ).toContain( + 'No cards matches tag filter /./ among 100 tags and image ' + + 'or histogram visualization filter.' + ); + })); + it( 'shows previous list when changed to malformed regex and it shows ' + 'the correct list when regex is fixed', diff --git a/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss b/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss index 1832737ede..b0c6fa5457 100644 --- a/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss +++ b/tensorboard/webapp/metrics/views/main_view/pinned_view_component.scss @@ -40,9 +40,5 @@ mat-icon { } .empty-message { - @include tb-theme-foreground-prop(color, secondary-text); - text-align: center; - padding: 16px; - font-size: 13px; - font-style: italic; + @include metrics-empty-message; }