From f3de1c71ab6ccd3a4cf28168dc7672340f980bc3 Mon Sep 17 00:00:00 2001 From: Saul Shanabrook Date: Wed, 19 Jun 2019 10:29:03 -0700 Subject: [PATCH 1/2] Removes table view in settings closes https://github.com/jupyterlab/jupyterlab/issues/6642 partially addresses https://github.com/jupyterlab/jupyterlab/issues/6623 --- packages/settingeditor/src/plugineditor.ts | 26 +--- packages/settingeditor/src/pluginlist.tsx | 44 +----- packages/settingeditor/src/settingeditor.tsx | 9 -- packages/settingeditor/src/tableeditor.tsx | 151 ------------------- packages/settingeditor/style/base.css | 32 +--- 5 files changed, 7 insertions(+), 255 deletions(-) delete mode 100644 packages/settingeditor/src/tableeditor.tsx diff --git a/packages/settingeditor/src/plugineditor.ts b/packages/settingeditor/src/plugineditor.ts index 4943f32de0c9..bbd36760b9d2 100644 --- a/packages/settingeditor/src/plugineditor.ts +++ b/packages/settingeditor/src/plugineditor.ts @@ -25,8 +25,6 @@ import { RawEditor } from './raweditor'; import { SettingEditor } from './settingeditor'; -import { TableEditor } from './tableeditor'; - /** * The class name added to all plugin editors. */ @@ -56,11 +54,9 @@ export class PluginEditor extends Widget { registry, rendermime }); - this.table = this._tableEditor = new TableEditor({ onSaveError }); this._rawEditor.handleMoved.connect(this._onStateChanged, this); layout.addWidget(this._rawEditor); - layout.addWidget(this._tableEditor); } /** @@ -68,16 +64,11 @@ export class PluginEditor extends Widget { */ readonly raw: RawEditor; - /** - * The plugin editor's table editor. - */ - readonly table: TableEditor; - /** * Tests whether the settings have been modified and need saving. */ get isDirty(): boolean { - return this._rawEditor.isDirty || this._tableEditor.isDirty; + return this._rawEditor.isDirty; } /** @@ -92,9 +83,8 @@ export class PluginEditor extends Widget { } const raw = this._rawEditor; - const table = this._tableEditor; - this._settings = raw.settings = table.settings = settings; + this._settings = raw.settings = settings; this.update(); } @@ -102,18 +92,16 @@ export class PluginEditor extends Widget { * The plugin editor layout state. */ get state(): SettingEditor.IPluginLayout { - const editor = this._editor; const plugin = this._settings ? this._settings.id : ''; const { sizes } = this._rawEditor; - return { editor, plugin, sizes }; + return { plugin, sizes }; } set state(state: SettingEditor.IPluginLayout) { if (JSONExt.deepEqual(this.state, state)) { return; } - this._editor = state.editor; this._rawEditor.sizes = state.sizes; this.update(); } @@ -154,7 +142,6 @@ export class PluginEditor extends Widget { super.dispose(); this._rawEditor.dispose(); - this._tableEditor.dispose(); } /** @@ -168,9 +155,7 @@ export class PluginEditor extends Widget { * Handle `'update-request'` messages. */ protected onUpdateRequest(msg: Message): void { - const editor = this._editor; const raw = this._rawEditor; - const table = this._tableEditor; const settings = this._settings; if (!settings) { @@ -179,8 +164,7 @@ export class PluginEditor extends Widget { } this.show(); - (editor === 'raw' ? table : raw).hide(); - (editor === 'raw' ? raw : table).show(); + raw.show(); } /** @@ -190,9 +174,7 @@ export class PluginEditor extends Widget { (this.stateChanged as Signal).emit(undefined); } - private _editor: 'raw' | 'table' = 'raw'; private _rawEditor: RawEditor; - private _tableEditor: TableEditor; private _settings: ISettingRegistry.ISettings | null = null; private _stateChanged = new Signal(this); } diff --git a/packages/settingeditor/src/pluginlist.tsx b/packages/settingeditor/src/pluginlist.tsx index 3f84693b8417..1839e54405f5 100644 --- a/packages/settingeditor/src/pluginlist.tsx +++ b/packages/settingeditor/src/pluginlist.tsx @@ -44,21 +44,6 @@ export class PluginList extends Widget { return this._changed; } - /** - * The editor type currently selected. - */ - get editor(): 'raw' | 'table' { - return this._editor; - } - set editor(editor: 'raw' | 'table') { - if (this._editor === editor) { - return; - } - - this._editor = editor; - this.update(); - } - /** * The selection value of the plugin list. */ @@ -120,10 +105,9 @@ export class PluginList extends Widget { */ protected onUpdateRequest(msg: Message): void { const { node, registry } = this; - const type = this._editor; const selection = this._selection; - Private.populateList(registry, type, selection, node); + Private.populateList(registry, selection, node); node.querySelector('ul').scrollTop = this._scrollTop; } @@ -142,15 +126,6 @@ export class PluginList extends Widget { return; } - const editor = target.getAttribute('data-editor'); - - if (editor) { - this._editor = editor as 'raw' | 'table'; - this._changed.emit(undefined); - this.update(); - return; - } - if (!id) { while (!id && target !== this.node) { target = target.parentElement as HTMLElement; @@ -176,7 +151,6 @@ export class PluginList extends Widget { private _changed = new Signal(this); private _confirm: () => Promise; - private _editor: 'raw' | 'table' = 'raw'; private _scrollTop = 0; private _selection = ''; } @@ -264,7 +238,6 @@ namespace Private { */ export function populateList( registry: ISettingRegistry, - type: 'raw' | 'table', selection: string, node: HTMLElement ): void { @@ -297,20 +270,7 @@ namespace Private { }); ReactDOM.unmountComponentAtNode(node); - ReactDOM.render( - -
- - -
-
    {items}
- , - node - ); + ReactDOM.render(
    {items}
, node); } /** diff --git a/packages/settingeditor/src/settingeditor.tsx b/packages/settingeditor/src/settingeditor.tsx index 03f5e2e46989..debcc1d699e8 100644 --- a/packages/settingeditor/src/settingeditor.tsx +++ b/packages/settingeditor/src/settingeditor.tsx @@ -248,7 +248,6 @@ export class SettingEditor extends Widget { private _onStateChanged(): void { this._state.sizes = this._panel.relativeSizes(); this._state.container = this._editor.state; - this._state.container.editor = this._list.editor; this._state.container.plugin = this._list.selection; this._saveState() .then(() => { @@ -329,7 +328,6 @@ export class SettingEditor extends Widget { panel.addWidget(editor); } editor.settings = settings; - list.editor = container.editor; list.selection = container.plugin; this._setLayout(); }) @@ -438,9 +436,6 @@ export namespace SettingEditor { * The current plugin being displayed. */ plugin: string; - - editor: 'raw' | 'table'; - sizes: number[]; } } @@ -496,10 +491,6 @@ namespace Private { : {}; saved.container = { - editor: - container.editor === 'raw' || container.editor === 'table' - ? container.editor - : DEFAULT_LAYOUT.container.editor, plugin: typeof container.plugin === 'string' ? container.plugin diff --git a/packages/settingeditor/src/tableeditor.tsx b/packages/settingeditor/src/tableeditor.tsx deleted file mode 100644 index 7f21a6a977f9..000000000000 --- a/packages/settingeditor/src/tableeditor.tsx +++ /dev/null @@ -1,151 +0,0 @@ -/*----------------------------------------------------------------------------- -| Copyright (c) Jupyter Development Team. -| Distributed under the terms of the Modified BSD License. -|----------------------------------------------------------------------------*/ - -import { ISettingRegistry } from '@jupyterlab/coreutils'; - -import { Message } from '@phosphor/messaging'; - -import { Widget } from '@phosphor/widgets'; - -import * as React from 'react'; - -import * as ReactDOM from 'react-dom'; - -/** - * A tabular editor for plugin settings. - */ -export class TableEditor extends Widget { - /** - * Create a new table editor for settings. - */ - constructor(options: TableEditor.IOptions) { - super({ node: document.createElement('fieldset') }); - this.addClass('jp-SettingsTableEditor'); - } - - /** - * Tests whether the settings have been modified and need saving. - */ - get isDirty(): boolean { - return false; // TODO: remove placeholder. - } - - /** - * The plugin settings. - */ - get settings(): ISettingRegistry.ISettings | null { - return this._settings; - } - set settings(settings: ISettingRegistry.ISettings | null) { - if (this._settings) { - this._settings.changed.disconnect(this._onSettingsChanged, this); - } - this._settings = settings; - if (this._settings) { - this._settings.changed.connect(this._onSettingsChanged, this); - } - this.update(); - } - - /** - * Handle `'update-request'` messages. - */ - protected onUpdateRequest(msg: Message): void { - const settings = this._settings; - - // Populate if possible. - if (settings) { - Private.populateTable(this.node, settings); - } - } - - /** - * Handle setting changes. - */ - private _onSettingsChanged(): void { - this.update(); - } - - private _settings: ISettingRegistry.ISettings | null = null; -} - -/** - * A namespace for `TableEditor` statics. - */ -export namespace TableEditor { - /** - * The instantiation options for a table editor. - */ - export interface IOptions { - /** - * A function the table editor calls on save errors. - */ - onSaveError: (reason: any) => void; - } -} - -/** - * A namespace for private module data. - */ -namespace Private { - /** - * Populate the fieldset with a specific plugin's metadata. - */ - export function populateTable( - node: HTMLElement, - settings: ISettingRegistry.ISettings - ): void { - const { plugin, schema } = settings; - const fields: { [property: string]: React.ReactElement } = {}; - const properties = schema.properties || {}; - const title = `(${plugin}) ${schema.description}`; - const label = `Fields - ${schema.title || plugin}`; - - Object.keys(properties).forEach(property => { - const field = properties[property]; - const { type } = field; - const defaultValue = settings.default(property); - const title = field.title || property; - const value = JSON.stringify(defaultValue) || ''; - const valueTitle = JSON.stringify(defaultValue, null, 4); - - fields[property] = ( - - - {property} - - - {value} - - {type} - - ); - }); - - const rows = Object.keys(fields) - .sort((a, b) => a.localeCompare(b)) - .map(property => fields[property]); - const fragment = ( - - {label} -
- - - - - - - - - {rows} -
KeyDefaultType
-
- - ); - - ReactDOM.unmountComponentAtNode(node); - ReactDOM.render(fragment, node); - } -} diff --git a/packages/settingeditor/style/base.css b/packages/settingeditor/style/base.css index 8a5e614262d2..748847d125de 100644 --- a/packages/settingeditor/style/base.css +++ b/packages/settingeditor/style/base.css @@ -8,7 +8,6 @@ --jp-private-settingeditor-key-width: 150px; --jp-private-settingeditor-legend-height: 16px; --jp-private-settingeditor-row-height: 16px; - --jp-private-settingeditor-switcher-height: 32px; --jp-private-settingeditor-toolbar-height: 28px; --jp-private-settingeditor-type-width: 75px; } @@ -66,35 +65,6 @@ width: 175px; } -#setting-editor .jp-PluginList-switcher { - height: var(--jp-private-settingeditor-switcher-height); - position: absolute; - top: 1px; - left: 0; - right: 0; - display: flex; -} - -#setting-editor .jp-PluginList-switcher button { - background: var(--jp-layout-color2); - color: var(--jp-ui-font-color2); - border: 0; - border-bottom: 1px solid var(--jp-ui-font-color3); - margin: 0; - padding: 0; - height: var(--jp-private-settingeditor-switcher-height); - flex: 1; -} - -#setting-editor .jp-PluginList-switcher button:first-child { - border-right: 1px solid var(--jp-ui-font-color3); -} - -#setting-editor .jp-PluginList-switcher button:disabled { - background: var(--jp-layout-color1); - color: var(--jp-ui-font-color0); -} - #setting-editor .jp-PluginList ul { background-color: var(--jp-layout-color1); color: var(--jp-ui-font-color1); @@ -104,7 +74,7 @@ padding: 0; overflow-y: auto; position: absolute; - top: var(--jp-private-settingeditor-switcher-height); + top: 0; bottom: 0; left: 0; right: 0; From f75896da281dca05722ec726f574f2e9471032e5 Mon Sep 17 00:00:00 2001 From: Saul Shanabrook Date: Wed, 19 Jun 2019 10:33:09 -0700 Subject: [PATCH 2/2] Added comments for layout --- packages/settingeditor/src/plugineditor.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/settingeditor/src/plugineditor.ts b/packages/settingeditor/src/plugineditor.ts index bbd36760b9d2..6b4fdcf6edef 100644 --- a/packages/settingeditor/src/plugineditor.ts +++ b/packages/settingeditor/src/plugineditor.ts @@ -44,6 +44,11 @@ export class PluginEditor extends Widget { this.addClass(PLUGIN_EDITOR_CLASS); const { commands, editorFactory, registry, rendermime } = options; + + // TODO: Remove this layout. We were using this before when we + // when we had a way to switch between the raw and table editor + // Now, the raw editor is the only child and probably could merged into + // this class directly in the future. const layout = (this.layout = new StackedLayout()); const { onSaveError } = Private;