Skip to content

Use UI toolkit for variable table #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"watch:src": "tsc -w --sourceMap"
},
"dependencies": {
"@jupyter/web-components": "^0.13.3",
"@jupyterlab/application": "^4.0.0",
"@jupyterlab/apputils": "^4.0.0",
"@jupyterlab/console": "^4.0.0",
Expand Down
174 changes: 115 additions & 59 deletions src/variableinspector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,25 @@ import { DockLayout, Widget } from '@lumino/widgets';

import { IVariableInspector } from './tokens';

import {
DataGrid as WebDataGrid,
DataGridRow,
DataGridCell,
allComponents,
provideJupyterDesignSystem,
Select,
Option,
Search,
Button
} from '@jupyter/web-components';
provideJupyterDesignSystem().register(allComponents);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should import the jp... variant of the components.

Then could import as fine grain the proper components to optimize what is loaded:

Suggested change
provideJupyterDesignSystem().register(allComponents);
provideJupyterDesignSystem().register(jpDataGrid(), jpDataGridRow(), ...);


import wildcardMatch from 'wildcard-match';

const TITLE_CLASS = 'jp-VarInspector-title';
const PANEL_CLASS = 'jp-VarInspector';
const TABLE_CLASS = 'jp-VarInspector-table';
const TABLE_BODY_CLASS = 'jp-VarInspector-content';
// const TABLE_BODY_CLASS = 'jp-VarInspector-content';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code?

Suggested change
// const TABLE_BODY_CLASS = 'jp-VarInspector-content';

const TABLE_ROW_CLASS = 'jp-VarInspector-table-row';
const TABLE_ROW_HIDDEN_CLASS = 'jp-VarInspector-table-row-hidden';
const TABLE_TYPE_CLASS = 'jp-VarInspector-type';
Expand All @@ -34,8 +47,8 @@ export class VariableInspectorPanel
implements IVariableInspector
{
private _source: IVariableInspector.IInspectable | null = null;
private _table: WebDataGrid;
private _filteredTable: HTMLDivElement;
private _table: HTMLTableElement;
private _title: HTMLElement;
private _filtered: { type: Array<string>; name: Array<string> };

Expand All @@ -58,13 +71,13 @@ export class VariableInspectorPanel
protected intializeFilteredTable() {
const filterType = this._filteredTable.querySelector(
'.' + FILTER_TYPE_CLASS
) as HTMLSelectElement;
) as Select;
const filterInput = this._filteredTable.querySelector(
'.' + FILTER_INPUT_CLASS
) as HTMLInputElement;
) as Search;
const filterButton = this._filteredTable.querySelector(
'.' + FILTER_BUTTON_CLASS
) as HTMLButtonElement;
) as Button;
filterButton.addEventListener('click', () => {
this.onFilterChange(
filterType.value as FILTER_TYPES,
Expand Down Expand Up @@ -137,14 +150,14 @@ export class VariableInspectorPanel
protected addFilteredOutRows() {
const rows = this._table.querySelectorAll(
'.' + TABLE_ROW_HIDDEN_CLASS
) as NodeListOf<HTMLTableRowElement>;
) as NodeListOf<DataGridRow>;
for (let i = 0; i < rows.length; i++) {
const rowName = rows[i].querySelector(
'.' + TABLE_NAME_CLASS
) as HTMLTableCellElement;
) as DataGridCell;
const rowType = rows[i].querySelector(
'.' + TABLE_TYPE_CLASS
) as HTMLTableCellElement;
) as DataGridCell;
if (
!this.stringInFilter(rowName.innerHTML, 'name') &&
!this._filtered['type'].includes(rowType.innerHTML)
Expand All @@ -161,14 +174,14 @@ export class VariableInspectorPanel
protected filterOutTable() {
const rows = this._table.querySelectorAll(
'.' + TABLE_ROW_CLASS
) as NodeListOf<HTMLTableRowElement>;
) as NodeListOf<DataGridRow>;
for (let i = 0; i < rows.length; i++) {
const rowName = rows[i].querySelector(
'.' + TABLE_NAME_CLASS
) as HTMLTableCellElement;
) as DataGridCell;
const rowType = rows[i].querySelector(
'.' + TABLE_TYPE_CLASS
) as HTMLTableCellElement;
) as DataGridCell;
if (
this.stringInFilter(rowName.innerHTML, 'name') ||
this._filtered['type'].includes(rowType.innerHTML)
Expand All @@ -178,6 +191,24 @@ export class VariableInspectorPanel
}
}

/*
Goes through each row and if it finds a variable with name 'name', then it deletes it
*/
protected removeRow(name: string) {
const rows = this._table.querySelectorAll(
'.' + TABLE_ROW_CLASS
) as NodeListOf<DataGridRow>;
for (let i = 0; i < rows.length; i++) {
const cell = rows[i].querySelector(
'.' + TABLE_NAME_CLASS
) as DataGridCell;
if (cell.innerHTML === name) {
rows[i].remove();
return;
}
}
}

get source(): IVariableInspector.IInspectable | null {
return this._source;
}
Expand Down Expand Up @@ -230,18 +261,28 @@ export class VariableInspectorPanel
" Inspecting '" + title.kernelName + "' " + title.contextName;
}

this._table.innerHTML = '';
const headerRow = document.createElement('jp-data-grid-row') as DataGridRow;
headerRow.className = 'sticky-header';
const columns = [' ', ' ', 'NAME', 'TYPE', 'SIZE', 'SHAPE', 'CONTENT'];
for (let i = 0; i < columns.length; i++) {
const headerCell = document.createElement(
'jp-data-grid-cell'
) as DataGridCell;
headerCell.className = 'column-header';
headerCell.textContent = columns[i];
headerCell.gridColumn = (i + 1).toString();
headerRow.appendChild(headerCell);
}
this._table.appendChild(headerRow);

//Render new variable state
let row: HTMLTableRowElement;
this._table.deleteTFoot();
this._table.createTFoot();
this._table.tFoot!.className = TABLE_BODY_CLASS;
for (let index = 0; index < args.length; index++) {
const item = args[index];

const name = item.varName;
const varType = item.varType;

row = this._table.tFoot!.insertRow();
const row = document.createElement('jp-data-grid-row') as DataGridRow;
row.className = TABLE_ROW_CLASS;
if (this._filtered['type'].includes(varType)) {
row.className = TABLE_ROW_HIDDEN_CLASS;
Expand All @@ -250,48 +291,62 @@ export class VariableInspectorPanel
}

// Add delete icon and onclick event
let cell = row.insertCell(0);
let cell = document.createElement('jp-data-grid-cell') as DataGridCell;
cell.title = 'Delete Variable';
cell.className = 'jp-VarInspector-deleteButton';
cell.gridColumn = '1';
const ico = closeIcon.element();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you wrap this icon in a jp-button with appearance stealth?

ico.className = 'icon-button';
ico.onclick = (ev: MouseEvent): any => {
this.source?.performDelete(name);
this.removeRow(name);
};
cell.append(ico);
row.appendChild(cell);

// Add onclick event for inspection
cell = row.insertCell(1);
cell = document.createElement('jp-data-grid-cell') as DataGridCell;
if (item.isMatrix) {
cell.title = 'View Contents';
cell.className = 'jp-VarInspector-inspectButton';
const ico = searchIcon.element();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind using a jp-button to wrap the icon in it (you can use the appearance stealth)?

ico.className = 'icon-button';
ico.onclick = (ev: MouseEvent): any => {
console.log('Click on ' + name);
console.log('Click on ' + item.varName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this

Suggested change
console.log('Click on ' + item.varName);

this._source
?.performMatrixInspection(name)
?.performMatrixInspection(item.varName)
.then((model: DataModel) => {
this._showMatrix(model, name, varType);
this._showMatrix(model, item.varName, item.varType);
});
};
cell.append(ico);
} else {
cell.innerHTML = '';
}
cell.gridColumn = '2';
row.appendChild(cell);

cell = row.insertCell(2);
cell = document.createElement('jp-data-grid-cell') as DataGridCell;
cell.className = TABLE_NAME_CLASS;
cell.innerHTML = name;
cell.gridColumn = '3';
row.appendChild(cell);

// Add remaining cells
cell = row.insertCell(3);
cell = document.createElement('jp-data-grid-cell') as DataGridCell;
cell.innerHTML = varType;
cell.className = TABLE_TYPE_CLASS;
cell = row.insertCell(4);
cell.gridColumn = '4';
row.appendChild(cell);
cell = document.createElement('jp-data-grid-cell') as DataGridCell;
cell.innerHTML = item.varSize;
cell = row.insertCell(5);
cell.gridColumn = '5';
row.appendChild(cell);
cell = document.createElement('jp-data-grid-cell') as DataGridCell;
cell.innerHTML = item.varShape;
cell = row.insertCell(6);
cell.gridColumn = '6';
row.appendChild(cell);

cell = document.createElement('jp-data-grid-cell') as DataGridCell;
const rendermime = this._source?.rendermime;
if (item.isWidget && rendermime) {
const model = new OutputAreaModel({ trusted: true });
Expand All @@ -304,6 +359,9 @@ export class VariableInspectorPanel
'</br>'
);
}
cell.gridColumn = '7';
row.appendChild(cell);
this._table.appendChild(row);
}
}

Expand Down Expand Up @@ -356,28 +414,22 @@ namespace Private {
);
}

export function createTable(): HTMLTableElement {
const table = document.createElement('table');
table.createTHead();
const hrow = table.tHead!.insertRow(0) as HTMLTableRowElement;

const cell1 = hrow.insertCell(0);
cell1.innerHTML = '';
const cell2 = hrow.insertCell(1);
cell2.innerHTML = '';
const cell3 = hrow.insertCell(2);
cell3.innerHTML = 'Name';
const cell4 = hrow.insertCell(3);
cell4.innerHTML = 'Type';
const cell5 = hrow.insertCell(4);
cell5.innerHTML = 'Size';
const cell6 = hrow.insertCell(5);
cell6.innerHTML = 'Shape';
const cell7 = hrow.insertCell(6);
cell7.innerHTML = 'Content';
export function createTable(): WebDataGrid {
const table = document.createElement('jp-data-grid') as WebDataGrid;
table.generateHeader = 'sticky';
table.gridTemplateColumns = '1fr 1fr 6fr 4fr 4fr 5fr 16fr';
return table;
}

export function createCellTemplate(table: WebDataGrid): HTMLTemplateElement {
const template = document.createElement('template');
const container = document.createElement('div');
container.innerText = 'testing';
template.appendChild(container);
table.appendChild(template);
return template;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code?

Suggested change
export function createCellTemplate(table: WebDataGrid): HTMLTemplateElement {
const template = document.createElement('template');
const container = document.createElement('div');
container.innerText = 'testing';
template.appendChild(container);
table.appendChild(template);
return template;
}

export function createTitle(header = ''): HTMLParagraphElement {
const title = document.createElement('p');
title.innerHTML = header;
Expand All @@ -387,27 +439,27 @@ namespace Private {
export function createFilterTable(): HTMLDivElement {
const container = document.createElement('div');
container.className = 'filter-container';
const filterType = document.createElement('select');
const filterType = document.createElement('jp-select') as Select;
filterType.className = FILTER_TYPE_CLASS;
filterType.selectedIndex = 0;
const varTypeOption = document.createElement('option');
const varTypeOption = document.createElement('jp-option') as Option;
varTypeOption.value = 'type';
varTypeOption.innerHTML = 'Type';
const nameOption = document.createElement('option');
const nameOption = document.createElement('jp-option') as Option;
nameOption.value = 'name';
nameOption.innerHTML = 'Name';
filterType.appendChild(varTypeOption);
filterType.appendChild(nameOption);
const searchContainer = document.createElement('div');
searchContainer.className = 'jp-InputGroup filter-search-container';
const input = document.createElement('input');
const input = document.createElement('jp-search') as Search;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jp-text-field will be more appropriate than the search component.

input.setAttribute('type', 'text');
input.setAttribute('placeholder', 'Filter out variable');
input.className = FILTER_INPUT_CLASS;
const filterButton = document.createElement('button');
const buttonText = document.createTextNode('Filter');
filterButton.appendChild(buttonText);
const filterButton = document.createElement('jp-button') as Button;
filterButton.textContent = 'Filter';
filterButton.className = FILTER_BUTTON_CLASS;
filterButton.appearance = 'accent';
const list = document.createElement('ul');
list.className = FILTER_LIST_CLASS;

Expand All @@ -423,18 +475,22 @@ namespace Private {
export function createFilteredButton(
filterName: string,
filterType: FILTER_TYPES
): HTMLButtonElement {
const filteredButton = document.createElement('button');
): Button {
const filteredButton = document.createElement('jp-button') as Button;
filteredButton.value = filterType;
filteredButton.title = filterType;
filteredButton.className = FILTERED_BUTTON_CLASS;
const filterButtonContent = document.createElement('div');
filterButtonContent.className = 'filter-button-content';
const buttonText = document.createElement('div');
buttonText.className = 'filtered-variable-button-text';
buttonText.innerHTML = filterName;
const icon = closeIcon.element({
container: filteredButton
container: filterButtonContent
});
filteredButton.appendChild(buttonText);
filteredButton.appendChild(icon);
filterButtonContent.appendChild(buttonText);
filterButtonContent.appendChild(icon);
filteredButton.appendChild(filterButtonContent);
filteredButton.className = FILTERED_BUTTON_CLASS;
return filteredButton;
}
Expand Down
Loading