Skip to content
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

Refine log console message UX #7448

Merged
merged 4 commits into from Nov 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 22 additions & 9 deletions packages/logconsole-extension/src/status.tsx
Expand Up @@ -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>
);
}
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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}
/>
);
}
Expand Down
18 changes: 18 additions & 0 deletions packages/logconsole/src/logger.ts
Expand Up @@ -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.
*/
Expand Down
17 changes: 15 additions & 2 deletions packages/logconsole/src/widget.ts
Expand Up @@ -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;
}

Expand Down
19 changes: 18 additions & 1 deletion packages/outputarea/src/model.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down