Skip to content

Commit

Permalink
Merge "ui: Remove old PopupMenu and replace with PopupMenu2" into main
Browse files Browse the repository at this point in the history
  • Loading branch information
stevegolton authored and Gerrit Code Review committed Nov 14, 2024
2 parents b642011 + 1ce711a commit ab428a4
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 343 deletions.
172 changes: 89 additions & 83 deletions ui/src/frontend/pivot_table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ import {
sliceAggregationColumns,
tables,
} from '../core/pivot_table_query_generator';
import {
PopupMenuButton,
popupMenuIcon,
PopupMenuItem,
} from '../widgets/popup_menu';
import {ReorderableCell, ReorderableCellGroup} from './reorderable_cells';
import {AttributeModalHolder} from './tables/attribute_modal_holder';
import {DurationWidget} from './widgets/duration';
Expand All @@ -49,6 +44,9 @@ import {argSqlColumn} from './widgets/sql/table/well_known_columns';
import {TraceImpl} from '../core/trace_impl';
import {PivotTableManager} from '../core/pivot_table_manager';
import {extensions} from '../public/lib/extensions';
import {MenuItem, PopupMenu2} from '../widgets/menu';
import {Button} from '../widgets/button';
import {popupMenuIcon} from '../widgets/table';

interface PathItem {
tree: PivotTree;
Expand Down Expand Up @@ -293,15 +291,14 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
return m('tr', overallValuesRow);
}

sortingItem(aggregationIndex: number, order: SortDirection): PopupMenuItem {
sortingItem(aggregationIndex: number, order: SortDirection): m.Child {
const pivotMgr = this.pivotMgr;
return {
itemType: 'regular',
text: order === 'DESC' ? 'Highest first' : 'Lowest first',
callback() {
return m(MenuItem, {
label: order === 'DESC' ? 'Highest first' : 'Lowest first',
onclick: () => {
pivotMgr.setSortColumn(aggregationIndex, order);
},
};
});
}

readableAggregationName(aggregation: Aggregation) {
Expand All @@ -317,20 +314,21 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
aggregation: Aggregation,
index: number,
nameOverride?: string,
): PopupMenuItem {
return {
itemType: 'regular',
text: nameOverride ?? readableColumnName(aggregation.column),
callback: () => this.pivotMgr.addAggregation(aggregation, index),
};
): m.Child {
return m(MenuItem, {
label: nameOverride ?? readableColumnName(aggregation.column),
onclick: () => {
this.pivotMgr.addAggregation(aggregation, index);
},
});
}

aggregationPopupTableGroup(
table: string,
columns: string[],
index: number,
): PopupMenuItem | undefined {
const items = [];
): m.Child | undefined {
const items: m.Child[] = [];
for (const column of columns) {
const tableColumn: TableColumn = {kind: 'regular', table, column};
items.push(
Expand All @@ -345,20 +343,15 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
return undefined;
}

return {
itemType: 'group',
itemId: `aggregations-${table}`,
text: `Add ${table} aggregation`,
children: items,
};
return m(MenuItem, {label: `Add ${table} aggregation`}, items);
}

renderAggregationHeaderCell(
aggregation: Aggregation,
index: number,
removeItem: boolean,
): ReorderableCell {
const popupItems: PopupMenuItem[] = [];
const popupItems: m.Child[] = [];
if (aggregation.sortDirection === undefined) {
popupItems.push(
this.sortingItem(index, 'DESC'),
Expand All @@ -381,22 +374,26 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
continue;
}
const pivotMgr = this.pivotMgr;
popupItems.push({
itemType: 'regular',
text: otherAgg,
callback() {
pivotMgr.setAggregationFunction(index, otherAgg);
},
});
popupItems.push(
m(MenuItem, {
label: otherAgg,
onclick: () => {
pivotMgr.setAggregationFunction(index, otherAgg);
},
}),
);
}
}

if (removeItem) {
popupItems.push({
itemType: 'regular',
text: 'Remove',
callback: () => this.pivotMgr.removeAggregation(index),
});
popupItems.push(
m(MenuItem, {
label: 'Remove',
onclick: () => {
this.pivotMgr.removeAggregation(index);
},
}),
);
}

let hasCount = false;
Expand Down Expand Up @@ -429,10 +426,15 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
extraClass: '.aggregation' + markFirst(index),
content: [
this.readableAggregationName(aggregation),
m(PopupMenuButton, {
icon: popupMenuIcon(aggregation.sortDirection),
items: popupItems,
}),
m(
PopupMenu2,
{
trigger: m(Button, {
icon: popupMenuIcon(aggregation.sortDirection),
}),
},
popupItems,
),
],
};
}
Expand All @@ -445,27 +447,27 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
selectedPivots: Set<string>,
): ReorderableCell {
const pivotMgr = this.pivotMgr;
const items: PopupMenuItem[] = [
{
itemType: 'regular',
text: 'Add argument pivot',
callback: () => {
const items: m.Child[] = [
m(MenuItem, {
label: 'Add argument pivot',
onclick: () => {
this.attributeModalHolder.start();
},
},
}),
];
if (queryResult.metadata.pivotColumns.length > 1) {
items.push({
itemType: 'regular',
text: 'Remove',
callback() {
pivotMgr.setPivotSelected({column: pivot, selected: false});
},
});
items.push(
m(MenuItem, {
label: 'Remove',
onclick: () => {
pivotMgr.setPivotSelected({column: pivot, selected: false});
},
}),
);
}

for (const table of tables) {
const group: PopupMenuItem[] = [];
const group: m.Child[] = [];
for (const columnName of table.columns) {
const column: TableColumn = {
kind: 'regular',
Expand All @@ -475,26 +477,30 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
if (selectedPivots.has(columnKey(column))) {
continue;
}
group.push({
itemType: 'regular',
text: columnName,
callback() {
pivotMgr.setPivotSelected({column, selected: true});
},
});
group.push(
m(MenuItem, {
label: columnName,
onclick: () => {
pivotMgr.setPivotSelected({column, selected: true});
},
}),
);
}
items.push({
itemType: 'group',
itemId: `pivot-${table.name}`,
text: `Add ${table.displayName} pivot`,
children: group,
});
items.push(
m(
MenuItem,
{
label: `Add ${table.displayName} pivot`,
},
group,
),
);
}

return {
content: [
readableColumnName(pivot),
m(PopupMenuButton, {icon: 'more_horiz', items}),
m(PopupMenu2, {trigger: m(Button, {icon: 'more_horiz'})}, items),
],
};
}
Expand Down Expand Up @@ -551,20 +557,20 @@ export class PivotTable implements m.ClassComponent<PivotTableAttrs> {
}),
m(
'td.menu',
m(PopupMenuButton, {
icon: 'menu',
items: [
{
itemType: 'regular',
text: state.constrainToArea
? 'Query data for the whole timeline'
: 'Constrain to selected area',
callback: () => {
this.pivotMgr.setConstrainedToArea(!state.constrainToArea);
},
m(
PopupMenu2,
{
trigger: m(Button, {icon: 'menu'}),
},
m(MenuItem, {
label: state.constrainToArea
? 'Query data for the whole timeline'
: 'Constrain to selected area',
onclick: () => {
this.pivotMgr.setConstrainedToArea(!state.constrainToArea);
},
],
}),
}),
),
),
),
),
Expand Down
18 changes: 12 additions & 6 deletions ui/src/frontend/value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

import m from 'mithril';
import {Tree, TreeNode} from '../widgets/tree';
import {PopupMenuButton, PopupMenuItem} from '../widgets/popup_menu';
import {PopupMenu2} from '../widgets/menu';
import {Button} from '../widgets/button';

// This file implements a component for rendering JSON-like values (with
// customisation options like context menu and action buttons).
Expand Down Expand Up @@ -109,7 +110,7 @@ type ButtonParams = {

// Customisation parameters which apply to any Value (e.g. context menu).
interface ValueParams {
contextMenu?: PopupMenuItem[];
contextMenu?: m.Child[];
}

// Customisation parameters which apply for a primitive value (e.g. showing
Expand Down Expand Up @@ -137,10 +138,15 @@ function renderValue(name: string, value: Value): m.Children {
const left = [
name,
value.contextMenu
? m(PopupMenuButton, {
icon: 'arrow_drop_down',
items: value.contextMenu,
})
? m(
PopupMenu2,
{
trigger: m(Button, {
icon: 'arrow_drop_down',
}),
},
value.contextMenu,
)
: null,
];
if (isArray(value)) {
Expand Down
25 changes: 0 additions & 25 deletions ui/src/plugins/dev.perfetto.WidgetsPage/widgets_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import {MultiParagraphText, TextParagraph} from '../../widgets/text_paragraph';
import {LazyTreeNode, Tree, TreeNode} from '../../widgets/tree';
import {VegaView} from '../../widgets/vega_view';
import {PageAttrs} from '../../public/page';
import {PopupMenuButton} from '../../widgets/popup_menu';
import {TableShowcase} from './table_showcase';
import {TreeTable, TreeTableAttrs} from '../../frontend/widgets/treetable';
import {Intent} from '../../widgets/common';
Expand Down Expand Up @@ -903,30 +902,6 @@ export class WidgetsPage implements m.ClassComponent<PageAttrs> {
repeatCheckedItemsAtTop: false,
},
}),
m(WidgetShowcase, {
label: 'PopupMenu',
renderWidget: () => {
return m(PopupMenuButton, {
icon: 'description',
items: [
{itemType: 'regular', text: 'New', callback: () => {}},
{itemType: 'regular', text: 'Open', callback: () => {}},
{itemType: 'regular', text: 'Save', callback: () => {}},
{itemType: 'regular', text: 'Delete', callback: () => {}},
{
itemType: 'group',
text: 'Share',
itemId: 'foo',
children: [
{itemType: 'regular', text: 'Friends', callback: () => {}},
{itemType: 'regular', text: 'Family', callback: () => {}},
{itemType: 'regular', text: 'Everyone', callback: () => {}},
],
},
],
});
},
}),
m(WidgetShowcase, {
label: 'Menu',
renderWidget: () =>
Expand Down
Loading

0 comments on commit ab428a4

Please sign in to comment.