May 25, 2018 · Apr 23, 2018 · Apr 24, 2018 · Apr 25, 2018 · Apr 26, 2018 · Apr 27, 2018
diff --git a/packages/apputils/src/mainareawidget.ts b/packages/apputils/src/mainareawidget.ts
     */
    ready?: Promise<void>;
  }

  /**
   * An options object for main area widget subclasses providing their own
   * default content.
   *
   * #### Notes
   * This makes it easier to have a subclass that provides its own default
   * content. This can go away once we upgrade to TypeScript 2.8 and have an
   * easy way to make a single property optional, ala
   * https://stackoverflow.com/a/46941824
   */
  export
  interface IOptionsOptionalContent<T extends Widget = Widget> extends Widget.IOptions {
    /**
     * The child widget to wrap.
     */
    content?: T;

    /**
     * The toolbar to use for the widget.  Defaults to an empty toolbar.
     */
    toolbar?: Toolbar;

    /**
     * An optional promise for when the content is ready to be shown.
     */
    ready?: Promise<void>;
  }

 }
diff --git a/packages/csvviewer/src/widget.ts b/packages/csvviewer/src/widget.ts
  }
 }

 export
 class CSVDocumentWidget extends DocumentWidget<CSVViewer> {
  constructor(options: CSVDocumentWidget.IOptions) {
    let {content, context, delimiter, ready, ...other} = options;
    content = content || Private.createContent(context);
    ready = Promise.all([ready, content.ready]).then(() => undefined );
    super({content, context, ready, ...other});

    if (delimiter) {
      content.delimiter = delimiter;
    }
    const csvDelimiter = new CSVDelimiter({ selected: content.delimiter });
    this.toolbar.addItem('delimiter', csvDelimiter);
    csvDelimiter.delimiterChanged.connect((sender: CSVDelimiter, delimiter: string) => { content.delimiter = delimiter; });
  }
 }


 export
 namespace CSVDocumentWidget {
  // TODO: In TypeScript 2.8, we can make just the content property optional
  // using something like https://stackoverflow.com/a/46941824, instead of
  // inheriting from this IOptionsOptionalContent.

  export
  interface IOptions extends DocumentWidget.IOptionsOptionalContent<CSVViewer> {
    delimiter?: string;
  }
 }

 namespace Private {
  export
  function createContent(context: DocumentRegistry.IContext<DocumentRegistry.IModel>) {
    return new CSVViewer({ context });
  }
 }

 /**
 * A widget factory for CSV widgets.
   * Create a new widget given a context.
   */
  protected createNewWidget(context: DocumentRegistry.Context): IDocumentWidget<CSVViewer> {
    const content = new CSVViewer({ context });
    const widget = new DocumentWidget({ content, context, ready: content.ready });
    const csvDelimiter = new CSVDelimiter({ selected: content.delimiter });
    widget.toolbar.addItem('delimiter', csvDelimiter);
    csvDelimiter.delimiterChanged.connect((sender: CSVDelimiter, delimiter: string) => { content.delimiter = delimiter; });
    return widget;
    return new CSVDocumentWidget({ context });
  }
 }
diff --git a/packages/docregistry/src/default.ts b/packages/docregistry/src/default.ts
  interface IOptions<T extends Widget = Widget, U extends DocumentRegistry.IModel = DocumentRegistry.IModel> extends MainAreaWidget.IOptions<T> {
      context: DocumentRegistry.IContext<U>;
  }

  export
  interface IOptionsOptionalContent<T extends Widget = Widget, U extends DocumentRegistry.IModel = DocumentRegistry.IModel> extends MainAreaWidget.IOptionsOptionalContent<T> {
      context: DocumentRegistry.IContext<U>;
  }
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -265,4 +265,33 @@ namespace MainAreaWidget {
		*/
		ready?: Promise<void>;
		}

		/**
		* An options object for main area widget subclasses providing their own
		* default content.
		*
		* #### Notes
		* This makes it easier to have a subclass that provides its own default
		* content. This can go away once we upgrade to TypeScript 2.8 and have an
		* easy way to make a single property optional, ala
		* https://stackoverflow.com/a/46941824
		*/
		export
		interface IOptionsOptionalContent<T extends Widget = Widget> extends Widget.IOptions {
		/**
		* The child widget to wrap.
		*/
		content?: T;
Copy link Member ian-r-roseMay 22, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. What is your current thinking on ownership issues for the content of a`MainAreaWidget`? When this is passed in, does ownership transfer to the main area widget, or does it stick with the`new` caller? Copy link ContributorAuthor jasongroutMay 23, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. It has to transfer ownership to the MainAreaWidget. In phosphor, widgets reparent/own their children, dispose their children, move their children in the DOM to be under them, etc.

		/**
		* The toolbar to use for the widget. Defaults to an empty toolbar.
		*/
		toolbar?: Toolbar;

		/**
		* An optional promise for when the content is ready to be shown.
		*/
		ready?: Promise<void>;
		}

		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -166,6 +166,42 @@ namespace CSVViewer {
		}
		}

		export
		class CSVDocumentWidget extends DocumentWidget<CSVViewer> {
Copy link Member ian-r-roseMay 24, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. These aren't particularly complex classes and interfaces, but they are missing docstrings for the API docs.
		constructor(options: CSVDocumentWidget.IOptions) {
		let {content, context, delimiter, ready, ...other} = options;
		content = content \|\| Private.createContent(context);
		ready = Promise.all([ready, content.ready]).then(() => undefined );
		super({content, context, ready, ...other});
Copy link Member ian-r-roseMay 22, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Did something change with TS? I thought that the`super` call had to be first in a constructor. Copy link ContributorAuthor jasongroutMay 23, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. The super has to be before you use any attributes of the object. Since I'm just creating local variables, it's fine. ian-r-rose reacted with thumbs up emoji

		if (delimiter) {
		content.delimiter = delimiter;
		}
		const csvDelimiter = new CSVDelimiter({ selected: content.delimiter });
		this.toolbar.addItem('delimiter', csvDelimiter);
		csvDelimiter.delimiterChanged.connect((sender: CSVDelimiter, delimiter: string) => { content.delimiter = delimiter; });
		}
		}


		export
		namespace CSVDocumentWidget {
		// TODO: In TypeScript 2.8, we can make just the content property optional
		// using something like https://stackoverflow.com/a/46941824, instead of
		// inheriting from this IOptionsOptionalContent.

		export
		interface IOptions extends DocumentWidget.IOptionsOptionalContent<CSVViewer> {
		delimiter?: string;
		}
		}

		namespace Private {
		export
		function createContent(context: DocumentRegistry.IContext<DocumentRegistry.IModel>) {
		return new CSVViewer({ context });
		}
		}

		/**
		* A widget factory for CSV widgets.
Expand All		@@ -176,11 +212,6 @@ class CSVViewerFactory extends ABCWidgetFactory<IDocumentWidget<CSVViewer>> {
		* Create a new widget given a context.
		*/
		protected createNewWidget(context: DocumentRegistry.Context): IDocumentWidget<CSVViewer> {
		const content = new CSVViewer({ context });
		const widget = new DocumentWidget({ content, context, ready: content.ready });
		const csvDelimiter = new CSVDelimiter({ selected: content.delimiter });
		widget.toolbar.addItem('delimiter', csvDelimiter);
		csvDelimiter.delimiterChanged.connect((sender: CSVDelimiter, delimiter: string) => { content.delimiter = delimiter; });
		return widget;
		return new CSVDocumentWidget({ context });
		}
		}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -475,4 +475,9 @@ namespace DocumentWidget {
		interface IOptions<T extends Widget = Widget, U extends DocumentRegistry.IModel = DocumentRegistry.IModel> extends MainAreaWidget.IOptions<T> {
		context: DocumentRegistry.IContext<U>;
		}

		export
		interface IOptionsOptionalContent<T extends Widget = Widget, U extends DocumentRegistry.IModel = DocumentRegistry.IModel> extends MainAreaWidget.IOptionsOptionalContent<T> {
		context: DocumentRegistry.IContext<U>;
		}
		}