Merge pull request #7448 from jasongrout/logstatus

Refine log console message UX
jupyterlab · Nov 1, 2019 · c27318d · c27318d
2 parents 25ceb49 + 050089e
commit c27318d
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 12 deletions.
diff --git a/packages/logconsole-extension/src/status.tsx b/packages/logconsole-extension/src/status.tsx
@@ -27,14 +27,15 @@ import React from 'react';
 function LogConsoleStatusComponent(
   props: LogConsoleStatusComponent.IProps
 ): React.ReactElement<LogConsoleStatusComponent.IProps> {
+  let title = '';
+  if (props.newMessages > 0) {
+    title = `${props.newMessages} new messages, `;
+  }
+  title += `${props.logEntries} log entries for ${props.source}`;
   return (
-    <GroupItem
-      spacing={0}
-      onClick={props.handleClick}
-      title={`${props.messages} messages in current log`}
-    >
+    <GroupItem spacing={0} onClick={props.handleClick} title={title}>
       <DefaultIconReact name={'list'} top={'2px'} kind={'statusBar'} />
-      <TextItem source={props.messages} />
+      {props.newMessages > 0 && <TextItem source={props.newMessages} />}
     </GroupItem>
   );
 }
@@ -54,9 +55,19 @@ namespace LogConsoleStatusComponent {
     handleClick: () => void;
 
     /**
-     * Number of log messages.
+     * Number of log entries.
+     */
+    logEntries: number;
+
+    /**
+     * Number of new log messages.
+     */
+    newMessages: number;
+
+    /**
+     * Log source name
      */
-    messages: number;
+    source: string;
   }
 }
 
@@ -106,7 +117,9 @@ export class LogConsoleStatus extends VDomRenderer<LogConsoleStatus.Model> {
     return (
       <LogConsoleStatusComponent
         handleClick={this._handleClick}
-        messages={messages}
+        logEntries={messages}
+        newMessages={version - versionDisplayed}
+        source={this.model.source}
       />
     );
   }

diff --git a/packages/logconsole/src/logger.ts b/packages/logconsole/src/logger.ts
@@ -131,6 +131,24 @@ export class LoggerOutputAreaModel extends OutputAreaModel
     return this.length;
   }
 
+  /**
+   * Whether an output should combine with the previous output.
+   *
+   * We combine if the two outputs are in the same second, which is the
+   * resolution for our time display.
+   */
+  protected shouldCombine(options: {
+    value: ILogOutput;
+    lastModel: ILogOutputModel;
+  }): boolean {
+    const { value, lastModel } = options;
+
+    let oldSeconds = Math.trunc(lastModel.timestamp.getTime() / 1000);
+    let newSeconds = Math.trunc(value.timestamp / 1000);
+
+    return oldSeconds === newSeconds;
+  }
+
   /**
    * Get an item at the specified index.
    */

diff --git a/packages/logconsole/src/widget.ts b/packages/logconsole/src/widget.ts
@@ -49,22 +49,35 @@ class LogConsoleOutputPrompt extends Widget implements IOutputPrompt {
    * Date & time when output is logged.
    */
   set timestamp(value: Date) {
-    this._timestampNode.innerHTML = value.toLocaleTimeString();
+    this._timestamp = value;
+    this._timestampNode.innerHTML = this._timestamp.toLocaleTimeString();
+    this.update();
   }
 
   /**
    * Log level
    */
   set level(value: FullLogLevel) {
+    this._level = value;
     this.node.dataset.logLevel = value;
-    this.node.title = `${toTitleCase(value)} message`;
+    this.update();
+  }
+
+  update() {
+    if (this._level !== undefined && this._timestamp !== undefined) {
+      this.node.title = `${this._timestamp.toLocaleString()}; ${toTitleCase(
+        this._level
+      )} level`;
+    }
   }
 
   /**
    * The execution count for the prompt.
    */
   executionCount: nbformat.ExecutionCount;
 
+  private _timestamp: Date;
+  private _level: FullLogLevel;
   private _timestampNode: HTMLDivElement;
 }
 

diff --git a/packages/outputarea/src/model.ts b/packages/outputarea/src/model.ts
@@ -307,7 +307,11 @@ export class OutputAreaModel implements IOutputAreaModel {
     if (
       nbformat.isStream(value) &&
       this._lastStream &&
-      value.name === this._lastName
+      value.name === this._lastName &&
+      this.shouldCombine({
+        value,
+        lastModel: this.list.get(this.length - 1)
+      })
     ) {
       // In order to get a list change event, we add the previous
       // text to the current item and replace the previous item.
@@ -342,6 +346,19 @@ export class OutputAreaModel implements IOutputAreaModel {
     return this.list.push(item);
   }
 
+  /**
+   * Whether a new value should be consolidated with the previous output.
+   *
+   * This will only be called if the minimal criteria of both being stream
+   * messages of the same type.
+   */
+  protected shouldCombine(options: {
+    value: nbformat.IOutput;
+    lastModel: IOutputModel;
+  }) {
+    return true;
+  }
+
   /**
    * A flag that is set when we want to clear the output area
    * *after* the next addition to it.