Skip to content
Open
Show file tree
Hide file tree
Changes from 17 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
24 changes: 21 additions & 3 deletions packages/console/src/Console.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
error?: string;
changes: DhType.ide.VariableChanges;
cancel: () => unknown;
startTimestamp?: DhType.LongWrapper;
endTimestamp?: DhType.LongWrapper;
};
}>
): void {
Expand Down Expand Up @@ -400,16 +402,22 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
message: string;
error?: string;
changes: DhType.ide.VariableChanges;
startTimestamp?: DhType.LongWrapper;
endTimestamp?: DhType.LongWrapper;
}
| undefined,
historyItem: ConsoleHistoryActionItem,
workspaceItemPromise: Promise<CommandHistoryStorageItem>
): void {
const serverStartTime = result?.startTimestamp?.asNumber();
const serverEndTime = result?.endTimestamp?.asNumber();
const newHistoryItem = {
...historyItem,
wrappedResult: undefined,
cancelResult: undefined,
result: result ?? historyItem.result,
serverStartTime,
serverEndTime,
};

this.setState(({ consoleHistory }) => {
Expand All @@ -432,7 +440,11 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
this.updateHistory(result, newHistoryItem);
this.updateKnownObjects(newHistoryItem);
this.updateWorkspaceHistoryItem(
{ error: result.error },
{
error: result.error,
serverStartTime,
serverEndTime,
},
workspaceItemPromise
);

Expand Down Expand Up @@ -655,11 +667,16 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {

/**
* Updates an existing workspace CommandHistoryItem
* @param result The result to store with the history item. Could be empty object for success
* @param result The result to store with the history item.
* Possibly contains an error message if there was a failure, server start time, and server end time.
* @param workspaceItemPromise The workspace data row promise for the workspace item to be updated
*/
updateWorkspaceHistoryItem(
result: { error?: string },
result: {
error?: string;
serverStartTime?: number;
serverEndTime?: number;
},
workspaceItemPromise: Promise<CommandHistoryStorageItem>
): void {
const promise = this.pending.add(workspaceItemPromise);
Expand Down Expand Up @@ -1107,6 +1124,7 @@ export class Console extends PureComponent<ConsoleProps, ConsoleState> {
supportsType={supportsType}
iconForType={iconForType}
ref={this.consoleHistoryContent}
onCommandSubmit={this.handleCommandSubmit}
/>
{historyChildren}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function makeCommandStarted() {
command: 'started',
result: null,
startTime: Date.now(),
endTime: Date.now(),
};
}

Expand Down Expand Up @@ -99,14 +100,14 @@ describe('different command results', () => {
expect(cleanup).toHaveBeenCalled();
});

it('renders <1s elapsed time for a command that was just started', () => {
it('renders correct time for a command that was just started', () => {
callback(makeCommandStarted());
expect(screen.getByText('<1s')).toBeTruthy();
expect(screen.getByText('0.00s')).toBeTruthy();
});

it('renders correct time for completed command', () => {
callback(makeCommandCompletedSuccess());
expect(screen.getByText('5s')).toBeTruthy();
expect(screen.getByText('5.00s')).toBeTruthy();
});

it('shows error message', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,6 @@ export class CommandHistoryItemTooltip extends Component<
onUpdate: (): void => undefined,
};

static getTimeString(
startTime: string | undefined,
endTime: string | number
): string | null {
if (startTime == null || endTime === '' || endTime === 0) {
return null;
}

const deltaTime = Math.round(
(new Date(endTime).valueOf() - new Date(startTime).valueOf()) / 1000
);

if (deltaTime < 1) return '<1s';

return TimeUtils.formatElapsedTime(deltaTime);
}

constructor(props: CommandHistoryItemTooltipProps) {
super(props);

Expand Down Expand Up @@ -167,7 +150,7 @@ export class CommandHistoryItemTooltip extends Component<

const errorMessage = result?.error ?? error;

const timeString = CommandHistoryItemTooltip.getTimeString(
const timeString = TimeUtils.formatConvertedDuration(
startTime,
endTime ?? currentTime
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface CommandHistoryStorageData {
command: string;
startTime: string;
endTime?: string;
result?: { error?: string };
result?: { error?: string; serverStartTime?: number; serverEndTime?: number };
}

export interface CommandHistoryStorageItem extends StorageItem {
Expand Down
8 changes: 8 additions & 0 deletions packages/console/src/console-history/ConsoleHistory.scss
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,11 @@ $console-gutter-width: 30px;
}
}
}

.console-history-item-tooltip {
display: grid;
grid-template-columns: auto auto;
gap: $spacer-1 $spacer-3;
text-align: left;
margin-bottom: $spacer-2;
}
3 changes: 3 additions & 0 deletions packages/console/src/console-history/ConsoleHistory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ interface ConsoleHistoryProps {
disabled?: boolean;
supportsType: (type: string) => boolean;
iconForType: (type: string) => ReactElement;
onCommandSubmit: (command: string) => void;
}

function itemKey(i: number, item: ConsoleHistoryActionItem): string {
Expand All @@ -37,6 +38,7 @@ const ConsoleHistory = React.forwardRef(function ConsoleHistory(
openObject,
supportsType,
iconForType,
onCommandSubmit,
} = props;
const historyElements = [];
for (let i = 0; i < items.length; i += 1) {
Expand All @@ -50,6 +52,7 @@ const ConsoleHistory = React.forwardRef(function ConsoleHistory(
language={language}
supportsType={supportsType}
iconForType={iconForType}
onCommandSubmit={onCommandSubmit}
/>
);
historyElements.push(historyElement);
Expand Down
68 changes: 68 additions & 0 deletions packages/console/src/console-history/ConsoleHistoryItem.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,38 @@ $button-vert-margin: ConsoleVariables.$button-vert-margin;
}

.console-history-item-command {
position: relative;
white-space: pre-line;
}

.console-history-actions {
position: absolute;
gap: $spacer-1;
top: 0;
right: $spacer-2;
display: none;
align-items: center;
background-color: var(--dh-color-neutral-hover-bg);
border-radius: $border-radius;
padding: $spacer-1;
}

.console-history-item-command-tooltip-active {
background-color: var(--dh-color-neutral-hover-bg);

.console-history-actions {
display: flex;
}
}

.console-history-item-command:hover {
background-color: var(--dh-color-neutral-hover-bg);

.console-history-actions {
display: flex;
}
}

.console-history-item-result .log-message {
white-space: pre-wrap;
word-wrap: break-word;
Expand Down Expand Up @@ -95,3 +124,42 @@ $button-vert-margin: ConsoleVariables.$button-vert-margin;
}
}
}

.console-history-actions-1 {
// actions are centered over the line
// note that the actions are much wider than the line itself
top: -($spacer-4 * 0.875);
}

.console-history-actions-2 {
// actions are centered over the line
top: -($spacer-1 * 0.5);
}

.console-command-result:last-child .console-history-actions-1 {
// pushed down so that the actions are visible
top: -$spacer-4;
}

.console-command-result:first-child .console-history-actions-1 {
// pushed up to prevent layout shifts
top: $spacer-0;
}

.console-history-item-contextual-help-content {
display: grid;
grid-template-columns: auto auto;
gap: $spacer-1 $spacer-3;
text-align: left;
margin-bottom: $spacer-2;
white-space: nowrap;
}

.console-history-item-contextual-help {
section[class*='spectrum-ContextualHelp-dialog'] {
// The default size of the contextual help is only 250px and is too small.
// Just set a size automatically based on the content.
width: fit-content;
min-width: 150px;
}
}
36 changes: 32 additions & 4 deletions packages/console/src/console-history/ConsoleHistoryItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import ConsoleHistoryResultInProgress from './ConsoleHistoryResultInProgress';
import ConsoleHistoryResultErrorMessage from './ConsoleHistoryResultErrorMessage';
import './ConsoleHistoryItem.scss';
import { type ConsoleHistoryActionItem } from './ConsoleHistoryTypes';
import ConsoleHistoryItemActions from './ConsoleHistoryItemActions';

const log = Log.module('ConsoleHistoryItem');

Expand All @@ -24,11 +25,16 @@ interface ConsoleHistoryItemProps {
// eslint-disable-next-line react/no-unused-prop-types
supportsType: (type: string) => boolean;
iconForType: (type: string) => ReactElement;
onCommandSubmit: (command: string) => void;
}

interface ConsoleHistoryItemState {
isTooltipVisible: boolean;
}

class ConsoleHistoryItem extends PureComponent<
ConsoleHistoryItemProps,
Record<string, never>
ConsoleHistoryItemState
> {
static defaultProps = {
disabled: false,
Expand All @@ -37,8 +43,13 @@ class ConsoleHistoryItem extends PureComponent<
constructor(props: ConsoleHistoryItemProps) {
super(props);

this.state = {
isTooltipVisible: false,
};

this.handleCancelClick = this.handleCancelClick.bind(this);
this.handleObjectClick = this.handleObjectClick.bind(this);
this.consoleHistoryItemClasses = this.consoleHistoryItemClasses.bind(this);
}

handleObjectClick(object: dh.ide.VariableDefinition): void {
Expand All @@ -56,18 +67,35 @@ class ConsoleHistoryItem extends PureComponent<
}
}

consoleHistoryItemClasses(): string {
const { isTooltipVisible } = this.state;
const classes = ['console-history-item-command'];
// console history items should stay highlighted if the tooltip is opened
if (isTooltipVisible) {
classes.push('console-history-item-command-tooltip-active');
}
return classes.join(' ');
}

render(): ReactElement {
const { disabled, item, language, iconForType } = this.props;
const { disabled, item, language, iconForType, onCommandSubmit } =
this.props;
const { disabledObjects, result } = item;
const hasCommand = item.command != null && item.command !== '';

let commandElement = null;
if (hasCommand) {
commandElement = (
<div className="console-history-item-command">
<div className={this.consoleHistoryItemClasses()}>
Copy link
Member

Choose a reason for hiding this comment

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

Just use classNames package instead of writing your own function.

Suggested change
<div className={this.consoleHistoryItemClasses()}>
<div className={classNames('console-history-item-command', { 'console-history-item-command-tooltip-active': isTootipVisible })>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

<div className="console-history-gutter">&gt;</div>
<div className="console-history-content">
<Code language={language}>{item.command}</Code>
<ConsoleHistoryItemActions
item={item}
onCommandSubmit={onCommandSubmit}
handleTooltipVisible={(isVisible: boolean) =>
this.setState({ isTooltipVisible: isVisible })
}
/>
</div>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import ConsoleHistoryItemActions from './ConsoleHistoryItemActions';

describe('clicking calls functionality', () => {
const user = userEvent.setup();
const handleCommandSubmit = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
return render(
<ConsoleHistoryItemActions
item={{ command: 'Test command' }}
onCommandSubmit={handleCommandSubmit}
handleTooltipVisible={jest.fn()}
/>
);
});

it('should render and rerun on click', async () => {
const button = screen.getAllByRole('button');
await user.click(button[1]);
expect(handleCommandSubmit).toHaveBeenCalledTimes(1);
});
});
Loading
Loading