Skip to content

Commit 0dfe8f4

Browse files
authored
Merge pull request #1292 from CleverCloud/cc-kv-terminal/virtualizer-test
fix(cc-kv-terminal): handle huge command history using lit-virtualizer
2 parents 7de9cd2 + 2b89f01 commit 0dfe8f4

File tree

2 files changed

+131
-31
lines changed

2 files changed

+131
-31
lines changed

src/components/cc-kv-terminal/cc-kv-terminal.js

Lines changed: 114 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import '@lit-labs/virtualizer';
12
import { css, html, LitElement } from 'lit';
23
import { classMap } from 'lit/directives/class-map.js';
34
import { createRef, ref } from 'lit/directives/ref.js';
@@ -10,9 +11,13 @@ import '../cc-icon/cc-icon.js';
1011

1112
/**
1213
* @typedef {import('./cc-kv-terminal.types.d.ts').CcKvTerminalState} CcKvTerminalState
14+
* @typedef {import('./cc-kv-terminal.types.d.ts').CcKvCommandContentItem} CcKvCommandContentItem
1315
* @typedef {import('../../lib/events.types.js').GenericEventWithTarget<KeyboardEvent,HTMLInputElement>} HTMLInputKeyboardEvent
1416
* @typedef {import('lit').PropertyValues<CcKvTerminal>} CcKvTerminalPropertyValues
17+
* @typedef {import('lit/directives/ref.js').Ref<HTMLDivElement>} HTMLDivElementRef
1518
* @typedef {import('lit/directives/ref.js').Ref<HTMLInputElement>} HTMLInputElementRef
19+
* @typedef {import('lit/directives/ref.js').Ref<Virtualizer>} VirtualizerRef
20+
* @typedef {import('@lit-labs/virtualizer/LitVirtualizer.js').LitVirtualizer} Virtualizer
1621
*/
1722

1823
/**
@@ -52,6 +57,19 @@ export class CcKvTerminal extends LitElement {
5257

5358
/** @type {HTMLInputElementRef} */
5459
this._promptRef = createRef();
60+
61+
/** @type {VirtualizerRef} */
62+
this._historyRef = createRef();
63+
64+
/** @type {HTMLDivElementRef} */
65+
this._scrollerRef = createRef();
66+
67+
// this is for lit-virtualizer
68+
this._elementRender = {
69+
/** @param {CcKvCommandContentItem} e */
70+
key: (e) => e.id,
71+
item: this._renderItem.bind(this),
72+
};
5573
}
5674

5775
/**
@@ -63,6 +81,60 @@ export class CcKvTerminal extends LitElement {
6381
}
6482
}
6583

84+
/**
85+
* @returns {Array<CcKvCommandContentItem>}
86+
*/
87+
_getItems() {
88+
/** @type {Array<CcKvCommandContentItem>} */
89+
const items = [];
90+
91+
this.state.history.forEach((historyEntry, cmdIndex) => {
92+
const cmdId = `cmd/${cmdIndex}`;
93+
const resultLength = historyEntry.result.length;
94+
95+
items.push({
96+
id: cmdId,
97+
type: 'commandLine',
98+
line: historyEntry.commandLine,
99+
hasResult: resultLength > 0,
100+
});
101+
102+
historyEntry.result.forEach((lineOfResult, resultIndex) => {
103+
items.push({
104+
id: `${cmdId}/result/${resultIndex}`,
105+
type: 'resultLine',
106+
line: lineOfResult,
107+
success: historyEntry.success,
108+
last: resultIndex === resultLength - 1,
109+
});
110+
});
111+
});
112+
113+
return items;
114+
}
115+
116+
/**
117+
* This is a strange workaround that makes sure we scroll to the bottom of a virtualizer
118+
*/
119+
async _scrollToBottom() {
120+
// We force scroll to the bottom.
121+
// This can be strange, but it is necessary because in some case, the `layoutComplete` promise that you see below doesn't resolve.
122+
// (When a promise doesn't resolve, the code after the `await` is never executed)
123+
// The `layoutComplete` promise doesn't resolve when the item added to the model won't generate a DOM node addition because it is too far from the current scroll position
124+
// This happens when user
125+
// 1. runs a command with a huge output
126+
// 2. scrolls up (enough to make the last item to be dropped by the virtualizer)
127+
// 3. hits `Enter`
128+
// Forcing a first scroll to bottom will force the `layoutComplete` promise to always resolve.
129+
this._scrollerRef.value.scrollTop = this._scrollerRef.value.scrollHeight;
130+
131+
// we wait for virtualizer to complete layout
132+
await this._historyRef.value?.layoutComplete;
133+
134+
// After the layout completes, we can do the real scroll to bottom
135+
this._scrollerRef.value.scrollTop = this._scrollerRef.value.scrollHeight;
136+
}
137+
66138
/**
67139
* @param {HTMLInputKeyboardEvent} e
68140
*/
@@ -124,6 +196,10 @@ export class CcKvTerminal extends LitElement {
124196
}
125197
}
126198
}
199+
200+
// When an input text receives this kind of event, the browser automatically scrolls to make it visible.
201+
// But to be pixel perfect, we force scroll to the bottom of the scroller.
202+
this._scrollerRef.value.scrollTop = this._scrollerRef.value.scrollHeight;
127203
}
128204

129205
/**
@@ -149,9 +225,11 @@ export class CcKvTerminal extends LitElement {
149225
/**
150226
* @param {CcKvTerminalPropertyValues} changedProperties
151227
*/
152-
updated(changedProperties) {
153-
if (changedProperties.has('state')) {
154-
this._promptRef.value?.scrollIntoView();
228+
async updated(changedProperties) {
229+
if (changedProperties.has('state') && this.state.type === 'idle') {
230+
await this._scrollToBottom();
231+
// don't ask me why, but we really need to call it twice to make sure it really scrolls to the bottom
232+
await this._scrollToBottom();
155233
}
156234
}
157235

@@ -168,23 +246,15 @@ export class CcKvTerminal extends LitElement {
168246
${i18n('cc-kv-terminal.warning')}
169247
</div>
170248
</div>
171-
<div class="content">
172-
${this.state.history.length > 0
173-
? html`
174-
<div aria-live="polite" aria-atomic="true">
175-
${this.state.history.map(({ commandLine, result, success }) => {
176-
return html`
177-
<div class="history-entry">
178-
<div class="history-entry-command">
179-
<cc-icon .icon=${iconShellPrompt}></cc-icon>${commandLine}
180-
</div>
181-
${result.map((l) => html`<div class=${classMap({ result: true, error: !success })}>${l}</div>`)}
182-
</div>
183-
`;
184-
})}
185-
</div>
186-
`
187-
: ''}
249+
<div class="scroller" ${ref(this._scrollerRef)}>
250+
<lit-virtualizer
251+
${ref(this._historyRef)}
252+
aria-live="polite"
253+
aria-atomic="true"
254+
.items=${this._getItems()}
255+
.keyFunction=${this._elementRender.key}
256+
.renderItem=${this._elementRender.item}
257+
></lit-virtualizer>
188258
<div class="prompt">
189259
<cc-icon .icon=${iconShellPrompt}></cc-icon>
190260
<label class="visually-hidden" for="prompt">${i18n('cc-kv-terminal.shell.prompt')}</label>
@@ -204,6 +274,22 @@ export class CcKvTerminal extends LitElement {
204274
`;
205275
}
206276

277+
/**
278+
* @param {CcKvCommandContentItem} item
279+
*/
280+
_renderItem(item) {
281+
switch (item.type) {
282+
case 'commandLine': {
283+
return html`<div class="command ${classMap({ empty: !item.hasResult })}">
284+
<cc-icon .icon=${iconShellPrompt}></cc-icon>${item.line}
285+
</div>`;
286+
}
287+
case 'resultLine': {
288+
return html`<div class="result ${classMap({ error: !item.success, last: item.last })}">${item.line}</div>`;
289+
}
290+
}
291+
}
292+
207293
static get styles() {
208294
return [
209295
accessibilityStyles,
@@ -219,8 +305,6 @@ export class CcKvTerminal extends LitElement {
219305
}
220306
221307
.wrapper {
222-
--shell-gap: 0.5em;
223-
224308
background-color: var(--cc-kv-terminal-color-background);
225309
color: var(--cc-kv-terminal-color-foreground);
226310
display: grid;
@@ -254,23 +338,17 @@ export class CcKvTerminal extends LitElement {
254338
gap: 0.5em;
255339
}
256340
257-
.content {
341+
.scroller {
258342
overflow: auto;
259343
padding: 0.5em;
260344
}
261345
262-
.history-entry {
263-
display: flex;
264-
flex-direction: column;
265-
gap: 0.2em;
266-
padding-bottom: var(--shell-gap);
267-
}
268-
269-
.history-entry-command {
346+
.command {
270347
align-items: center;
271348
display: flex;
272349
font-weight: bold;
273350
gap: 0.2em;
351+
padding-bottom: 0.2em;
274352
}
275353
276354
cc-icon {
@@ -287,6 +365,11 @@ export class CcKvTerminal extends LitElement {
287365
color: var(--cc-kv-terminal-color-foreground-error);
288366
}
289367
368+
.result.last,
369+
.command.empty {
370+
padding-bottom: 0.5em;
371+
}
372+
290373
.prompt {
291374
align-items: center;
292375
display: flex;

src/components/cc-kv-terminal/cc-kv-terminal.types.d.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,20 @@ export interface CcKvCommandHistoryEntry {
1616
result: Array<string>;
1717
success: boolean;
1818
}
19+
20+
export type CcKvCommandContentItem = CcKvCommandContentItemCommandLine | CcKvCommandContentItemResultLine;
21+
22+
interface CcKvCommandContentItemCommandLine {
23+
id: string;
24+
type: 'commandLine';
25+
line: string;
26+
hasResult: boolean;
27+
}
28+
29+
interface CcKvCommandContentItemResultLine {
30+
id: string;
31+
type: 'resultLine';
32+
line: string;
33+
success: boolean;
34+
last: boolean;
35+
}

0 commit comments

Comments
 (0)