Skip to content

Commit

Permalink
Move starring interaction from app toolbar to per-table data entry.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 482540331
  • Loading branch information
@cjqian
cjqian authored and LIT team committed Oct 20, 2022
1 parent 4851c9d commit cd14f35
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 70 deletions.
35 changes: 0 additions & 35 deletions lit_nlp/client/core/main_toolbar.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {MenuItem} from '../elements/menu';
import {styles as sharedStyles} from '../lib/shared_styles.css';
import {compareArrays, flatten, randInt, shortenId} from '../lib/utils';
import {AppState, ColorService, SelectionService, SliceService} from '../services/services';
import {STARRED_SLICE_NAME} from '../services/slice_service';

import {app} from './app';
import {styles} from './main_toolbar.css';
Expand Down Expand Up @@ -222,7 +221,6 @@ export class LitMainToolbar extends MobxLitElement {
private readonly pinnedSelectionService =
app.getService(SelectionService, 'pinned');
private readonly selectionService = app.getService(SelectionService);
private readonly sliceService = app.getService(SliceService);

/**
* ID pairs, as [child, parent], from current selection or the whole dataset.
Expand All @@ -234,20 +232,6 @@ export class LitMainToolbar extends MobxLitElement {
.map(d => [d.id, d.meta['parentId']!]);
}

private isStarred(id: string|null): boolean {
return (id !== null) && this.sliceService.isInSlice(STARRED_SLICE_NAME, id);
}

private toggleStarred() {
const primaryId = this.selectionService.primarySelectedId;
if (primaryId == null) return;
if (this.isStarred(primaryId)) {
this.sliceService.removeIdsFromSlice(STARRED_SLICE_NAME, [primaryId]);
} else {
this.sliceService.addIdsToSlice(STARRED_SLICE_NAME, [primaryId]);
}
}

/**
* Button to select a random example.
*/
Expand Down Expand Up @@ -425,24 +409,6 @@ export class LitMainToolbar extends MobxLitElement {
// clang-format on
}

renderStarButton(numSelected: number) {
const highlightStar =
this.isStarred(this.selectionService.primarySelectedId);

const disabled = numSelected === 0;
const iconClass = classMap({'icon-button': true, 'disabled': disabled});

const starOnClick = () => {
this.toggleStarred();
};
// clang-format off
return html`
<mwc-icon class=${iconClass} id='star-button' @click=${starOnClick}>
${highlightStar ? 'star' : 'star_border'}
</mwc-icon>`;
// clang-format on
}

override render() {
const clearSelection = () => {
this.selectionService.selectIds([]);
Expand Down Expand Up @@ -506,7 +472,6 @@ export class LitMainToolbar extends MobxLitElement {
</div>
<div id='right-container'>
${primaryId !== null ? this.renderPrimarySelectControls() : null}
${this.renderStarButton(numSelected)}
${this.renderSelectionDisplay(numSelected, numTotal)}
<button id="select-all" class="hairline-button xl"
@click=${selectAll}>
Expand Down
11 changes: 9 additions & 2 deletions lit_nlp/client/elements/table.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ import {styles} from './table.css';
/** Function for supplying table entry template result based on row state. */
export type TemplateResultFn =
(isSelected: boolean, isPrimarySelection: boolean,
isReferenceSelection: boolean, isFocused: boolean) => TemplateResult;
isReferenceSelection: boolean, isFocused: boolean, isStarred: boolean) =>
TemplateResult;

type SortableTableEntry = string|number;
/** Wrapper type for sortable custom data table entries */
Expand Down Expand Up @@ -122,6 +123,7 @@ export class DataTable extends ReactiveElement {
@observable.struct @property({type: Array})
columnNames: Array<string|ColumnHeader> = [];
@observable.struct @property({type: Array}) selectedIndices: number[] = [];
@observable.struct @property({type: Array}) starredIndices: number[] = [];
@observable @property({type: Number}) primarySelectedIndex: number = -1;
@observable @property({type: Number}) referenceSelectedIndex: number = -1;
// TODO(lit-dev): consider a custom reaction to make this more responsive,
Expand Down Expand Up @@ -912,6 +914,7 @@ export class DataTable extends ReactiveElement {
const isPrimarySelection = this.primarySelectedIndex === dataIndex;
const isReferenceSelection = this.referenceSelectedIndex === dataIndex;
const isFocused = this.focusedIndex === dataIndex;
const isStarred = this.starredIndices.indexOf(dataIndex) !== -1;
const rowClass = classMap({
'lit-data-table-row': true,
'selected': isSelected,
Expand Down Expand Up @@ -949,7 +952,11 @@ export class DataTable extends ReactiveElement {
const t = (d as SortableTemplateResult).template;
if (typeof t === 'function') {
templateResult = t(
isSelected, isPrimarySelection, isReferenceSelection, isFocused);
isSelected,
isPrimarySelection,
isReferenceSelection,
isFocused,
isStarred);
} else {
templateResult = t;
}
Expand Down
122 changes: 89 additions & 33 deletions lit_nlp/client/modules/data_table_module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ import {BooleanLitType, LitType, LitTypeWithVocab} from '../lib/lit_types';
import {styles as sharedStyles} from '../lib/shared_styles.css';
import {formatForDisplay, IndexedInput, ModelInfoMap, Spec} from '../lib/types';
import {compareArrays} from '../lib/utils';
import {DataService, FocusService, SelectionService} from '../services/services';
import {DataService, FocusService, SelectionService, SliceService} from '../services/services';
import {STARRED_SLICE_NAME} from '../services/slice_service';

import {styles} from './data_table_module.css';

Expand All @@ -59,6 +60,7 @@ export class DataTableModule extends LitModule {

private readonly focusService = app.getService(FocusService);
private readonly dataService = app.getService(DataService);
private readonly sliceService = app.getService(SliceService);
private readonly referenceSelectionService =
app.getService(SelectionService, 'pinned');

Expand All @@ -83,7 +85,9 @@ export class DataTableModule extends LitModule {
get keys(): ColumnHeader[] {
function createColumnHeader(name: string, type: LitType) {
const header = {name, vocab: (type as LitTypeWithVocab).vocab};
if (type instanceof BooleanLitType) {header.vocab = ['✔', ' '];}
if (type instanceof BooleanLitType) {
header.vocab = ['✔', ' '];
}
return header;
}

Expand Down Expand Up @@ -186,6 +190,15 @@ export class DataTableModule extends LitModule {
return -1;
}

@computed
get starredIndices(): number[] {
const starredIds = this.sliceService.getSliceByName(STARRED_SLICE_NAME);
if (starredIds) {
return starredIds.map(sid => this.indexOfId(sid));
}
return [];
}

@computed
get focusedIndex(): number {
// Set focused index if a datapoint is focused according to the focus
Expand All @@ -210,6 +223,20 @@ export class DataTableModule extends LitModule {
.reverse();
}

private isStarred(id: string|null): boolean {
return (id !== null) && this.sliceService.isInSlice(STARRED_SLICE_NAME, id);
}

private toggleStarred(id: string|null) {
if (id == null) return;
if (this.isStarred(id)) {
this.sliceService.removeIdsFromSlice(STARRED_SLICE_NAME, [id]);
} else {
this.sliceService.addIdsToSlice(STARRED_SLICE_NAME, [id]);
}
}


// TODO(lit-dev): figure out why this updates so many times;
// it gets run _four_ times every time a new datapoint is added.
@computed
Expand All @@ -233,51 +260,79 @@ export class DataTableModule extends LitModule {
event.stopPropagation();
};

const starClick = (event: Event) => {
this.toggleStarred(d.id);
event.stopPropagation();
event.preventDefault();
};

// Provide a template function for the 'index' column so that the
// rendering can be based on the selection/hover state of the datapoint
// represented by the row.
function templateFn(isSelected: boolean, isPrimarySelection: boolean,
isReferenceSelection: boolean, isFocused: boolean) {
const indexHolderDivStyle = styleMap({
'display': 'flex',
'flex-direction': 'row-reverse',
'justify-content': 'space-between',
'width': '100%'
function templateFn(
isSelected: boolean, isPrimarySelection: boolean,
isReferenceSelection: boolean, isFocused: boolean,
isStarred: boolean) {
const indexHolderDivStyle = styleMap({
'display': 'flex',
'justify-content': 'space-between',
'width': '100%'
});
const indexDivStyle = styleMap({
'text-align': 'right',
});
// Render the action button next to the index if datapoint is selected,
// hovered, or active (pinned, starred).
function renderActionButtons() {
function getActionStyle(isActive: boolean) {
return styleMap({
'visibility': isPrimarySelection || isFocused || isActive ?
'visible' :
'hidden',
});
const indexDivStyle = styleMap({
'text-align': 'right',
}

function getActionClass(isActive: boolean) {
return classMap({
'icon-button': true,
'cyea': true,
'mdi-outlined': !isActive,
});
// Render the pin button next to the index if datapoint is pinned,
// selected, or hovered.
function renderPin() {
const iconClass = classMap({
'icon-button': true,
'cyea': true,
'mdi-outlined': !isReferenceSelection,
});
if (isReferenceSelection || isPrimarySelection || isFocused) {
return html`
<mwc-icon class="${iconClass}" @click=${pinClick}>
push_pin
</mwc-icon>`;
}
return null;
}
return html`
}

return html`
<mwc-icon style="${
getActionStyle(isReferenceSelection)}" class="${
getActionClass(isReferenceSelection)}" @click=${pinClick}
title=${
isReferenceSelection ? 'Pin datapoint' : 'Unpin datapoint'}>
push_pin
</mwc-icon>
<mwc-icon style="${getActionStyle(isStarred)}" class="${
getActionClass(isStarred)}" @click=${starClick}
title=${
isStarred ? 'Remove from starred slice' :
'Add to starred slice'}>
${isStarred ? 'star' : 'star_border'}
</mwc-icon>`;
}
return html`
<div style="${indexHolderDivStyle}">
${renderActionButtons()}
<div style="${indexDivStyle}">${index}</div>
${renderPin()}
</div>`;
}
}
const indexEntry = {template: templateFn, value: index};
return [indexEntry, ...dataEntry];
});
}

override firstUpdated() {
const updateColsChange = () => [
this.appState.currentModels, this.appState.currentDataset, this.keys];
this.reactImmediately(updateColsChange, () => {this.updateColumns();});
const updateColsChange = () =>
[this.appState.currentModels, this.appState.currentDataset, this.keys];
this.reactImmediately(updateColsChange, () => {
this.updateColumns();
});
}

private updateColumns() {
Expand Down Expand Up @@ -412,6 +467,7 @@ export class DataTableModule extends LitModule {
.selectedIndices=${this.selectedRowIndices}
.primarySelectedIndex=${this.primarySelectedIndex}
.referenceSelectedIndex=${this.referenceSelectedIndex}
.starredIndices=${this.starredIndices}
.focusedIndex=${this.focusedIndex}
.onSelect=${(idxs: number[]) => { this.onSelect(idxs); }}
.onPrimarySelect=${(i: number) => { this.onPrimarySelect(i); }}
Expand Down

0 comments on commit cd14f35

Please sign in to comment.