Skip to content

Commit d6f8eda

Browse files
sandy081Copilot
andauthored
sessions: Add spacing between session list rows (#333138)
* sessions: add spacing between session list rows Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: update chat row height assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 227803b commit d6f8eda

3 files changed

Lines changed: 111 additions & 9 deletions

File tree

src/vs/sessions/contrib/sessions/browser/media/sessionsList.css

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,47 @@
1414
width: calc(100% - 20px);
1515
}
1616

17+
&.session-list-row-spacing .monaco-list-row.session-list-inset-row {
18+
padding-bottom: var(--vscode-spacing-size20);
19+
background-clip: content-box;
20+
outline: 0 !important;
21+
22+
> .monaco-tl-row {
23+
border-radius: inherit;
24+
}
25+
}
26+
27+
&.session-list-row-spacing .monaco-list .monaco-list-row.session-list-inset-row {
28+
&.selected > .monaco-tl-row {
29+
outline: var(--vscode-strokeThickness) dotted var(--vscode-contrastActiveBorder, transparent);
30+
outline-offset: calc(-1 * var(--vscode-strokeThickness));
31+
}
32+
33+
&.focused > .monaco-tl-row {
34+
outline: var(--vscode-strokeThickness) dotted var(--vscode-list-inactiveFocusOutline, transparent);
35+
outline-offset: calc(-1 * var(--vscode-strokeThickness));
36+
}
37+
38+
&.focused.selected > .monaco-tl-row {
39+
outline-color: var(--vscode-contrastActiveBorder, var(--vscode-list-inactiveFocusOutline, transparent));
40+
}
41+
42+
&:hover > .monaco-tl-row {
43+
outline: var(--vscode-strokeThickness) dashed var(--vscode-contrastActiveBorder, transparent);
44+
outline-offset: calc(-1 * var(--vscode-strokeThickness));
45+
}
46+
}
47+
48+
&.session-list-row-spacing .monaco-list:focus .monaco-list-row.session-list-inset-row.focused > .monaco-tl-row,
49+
.context-menu-visible &.session-list-row-spacing .monaco-list.last-focused .monaco-list-row.session-list-inset-row.focused > .monaco-tl-row {
50+
outline: var(--vscode-strokeThickness) solid var(--vscode-list-focusOutline);
51+
outline-offset: calc(-1 * var(--vscode-strokeThickness));
52+
}
53+
54+
&.session-list-row-spacing .monaco-list:focus .monaco-list-row.session-list-inset-row.focused.selected > .monaco-tl-row {
55+
outline-color: var(--vscode-list-focusAndSelectionOutline, var(--vscode-contrastActiveBorder, var(--vscode-list-focusOutline)));
56+
}
57+
1758
/* Suppressed while selected so the selection highlight is not overpainted. */
1859
.monaco-list-row:not(.selected):not(.focused):not(:hover):has(.session-item.needs-input),
1960
.monaco-list-row:not(.selected):not(.focused):not(:hover) .session-chat-item.needs-input {

src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ function isSessionItem(item: SessionListItem): item is ISession {
261261

262262
const SHOW_MORE_FOLDERS_LABEL = '__more_folders__';
263263
const FOUR_DAYS_MS = 4 * 24 * 60 * 60 * 1000;
264+
const INSET_ROW_GAP = 2;
264265

265266
/**
266267
* Default number of terminal-command lines shown in a session row's approval
@@ -311,8 +312,13 @@ class SessionsTreeDelegate implements IListVirtualDelegate<SessionListItem> {
311312
* flat, chat-less lists (blocked sessions, automations) set it `true`.
312313
*/
313314
private readonly _aggregateChatApprovals = false,
315+
private readonly _useInsetRowSpacing = false,
314316
) { }
315317

318+
private withInsetRowSpacing(height: number): number {
319+
return height + (this._useInsetRowSpacing ? INSET_ROW_GAP : 0);
320+
}
321+
316322
getHeight(element: SessionListItem): number {
317323
if (isSessionChatItem(element)) {
318324
let chatHeight = this._isPhone() ? SessionsTreeDelegate.CHAT_ITEM_HEIGHT_PHONE : SessionsTreeDelegate.CHAT_ITEM_HEIGHT;
@@ -326,13 +332,13 @@ class SessionsTreeDelegate implements IListVirtualDelegate<SessionListItem> {
326332
chatHeight += SessionItemRenderer.getApprovalRowHeight(approval.label, this._approvalRowMaxLines) + SessionsTreeDelegate.CHAT_APPROVAL_BOTTOM_SLACK;
327333
}
328334
}
329-
return chatHeight;
335+
return this.withInsetRowSpacing(chatHeight);
330336
}
331337
if (isSessionSection(element) || isSessionGroupItem(element)) {
332338
return SessionsTreeDelegate.SECTION_HEIGHT;
333339
}
334340
if (isSessionShowMore(element)) {
335-
return SessionsTreeDelegate.SHOW_MORE_HEIGHT;
341+
return this.withInsetRowSpacing(SessionsTreeDelegate.SHOW_MORE_HEIGHT);
336342
}
337343
if (isSessionPlaceholder(element)) {
338344
return SessionsTreeDelegate.PLACEHOLDER_HEIGHT;
@@ -358,7 +364,7 @@ class SessionsTreeDelegate implements IListVirtualDelegate<SessionListItem> {
358364
if (this._ciFixModel && this._ciFixModel.getCIFix(element as ISession).get()) {
359365
height += SessionItemRenderer.CI_ROW_HEIGHT;
360366
}
361-
return height;
367+
return this.withInsetRowSpacing(height);
362368
}
363369

364370
hasDynamicHeight(element: SessionListItem): boolean {
@@ -2413,7 +2419,7 @@ export class SessionsList extends Disposable implements ISessionsList {
24132419
this._excludeRead = this.storageService.getBoolean(SessionsList.EXCLUDE_READ_KEY, StorageScope.PROFILE, false);
24142420
this.workspaceGroupCapped = this.storageService.getBoolean(SessionsList.WORKSPACE_GROUP_CAPPED_KEY, StorageScope.PROFILE, true);
24152421

2416-
this.listContainer = DOM.append(container, $('.sessions-list-control'));
2422+
this.listContainer = DOM.append(container, $('.sessions-list-control.session-list-row-spacing'));
24172423
this._register(DOM.addDisposableListener(this.listContainer, DOM.EventType.POINTER_DOWN, () => {
24182424
this.listContainer.classList.add(SESSION_SECTION_FOCUS_FROM_POINTER_CLASS);
24192425
}));
@@ -2507,7 +2513,15 @@ export class SessionsList extends Disposable implements ISessionsList {
25072513
// observe the workbench's value rather than shadowing it with a fresh
25082514
// scoped default of `false`. The reactive height refresh below listens
25092515
// on the same scoped service for changes.
2510-
const delegate = new SessionsTreeDelegate(approvalModel, () => !!IsPhoneLayoutContext.getValue(contextKeyService));
2516+
const delegate = new SessionsTreeDelegate(
2517+
approvalModel,
2518+
() => !!IsPhoneLayoutContext.getValue(contextKeyService),
2519+
DEFAULT_APPROVAL_ROW_MAX_LINES,
2520+
undefined,
2521+
true /* useCompactQuickChatRows */,
2522+
false /* aggregateChatApprovals */,
2523+
true /* useInsetRowSpacing */,
2524+
);
25112525
this._delegate = delegate;
25122526

25132527
this.tree = this._register(instantiationService.createInstance(

src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,53 @@ suite('Sessions - SessionsList', () => {
920920
});
921921
});
922922

923+
suite('session row spacing', () => {
924+
test('reserves spacing only in the main sessions list', () => {
925+
const sessions = [
926+
createTestSession('First').session,
927+
createTestSession('Second').session,
928+
];
929+
930+
const mainHarness = createListHarness(disposables, sessions);
931+
const mainContainer = mainHarness.createContainer();
932+
const mainList = mainHarness.store.add(mainHarness.instantiationService.createInstance(SessionsList, mainContainer, {
933+
grouping: () => SessionsGrouping.Date,
934+
sorting: () => SessionsSorting.Created,
935+
onSessionOpen: () => { },
936+
}));
937+
mainList.layout(300, 400);
938+
939+
const flatHarness = createListHarness(disposables, sessions);
940+
const flatContainer = flatHarness.createContainer();
941+
const flatList = flatHarness.store.add(flatHarness.instantiationService.createInstance(SessionsFlatList, flatContainer, {
942+
showSessionHover: false,
943+
onSessionOpen: () => { },
944+
}));
945+
flatList.setSessions(sessions);
946+
flatList.layout(300, 400);
947+
948+
const mainRows = [...mainContainer.querySelectorAll<HTMLElement>('.session-item')]
949+
.map(item => item.closest<HTMLElement>('.monaco-list-row')!);
950+
const flatRows = [...flatContainer.querySelectorAll<HTMLElement>('.session-item')]
951+
.map(item => item.closest<HTMLElement>('.monaco-list-row')!);
952+
assert.deepStrictEqual({
953+
mainHasSpacingClass: mainContainer.querySelector('.sessions-list-control')?.classList.contains('session-list-row-spacing'),
954+
mainRowHeight: mainRows[0].style.height,
955+
mainRowOffset: parseInt(mainRows[1].style.top) - parseInt(mainRows[0].style.top),
956+
flatHasSpacingClass: flatContainer.querySelector('.sessions-list-control')?.classList.contains('session-list-row-spacing'),
957+
flatRowHeight: flatRows[0].style.height,
958+
flatRowOffset: parseInt(flatRows[1].style.top) - parseInt(flatRows[0].style.top),
959+
}, {
960+
mainHasSpacingClass: true,
961+
mainRowHeight: '56px',
962+
mainRowOffset: 56,
963+
flatHasSpacingClass: false,
964+
flatRowHeight: '54px',
965+
flatRowOffset: 54,
966+
});
967+
});
968+
});
969+
923970
suite('session chat rows', () => {
924971

925972
function createChat(title: string, origin?: ChatOriginKind, interactivity = ChatInteractivity.Full, status = SessionStatus.Completed): IChat {
@@ -1218,8 +1265,8 @@ suite('Sessions - SessionsList', () => {
12181265
assert.ok(phoneChatRow);
12191266

12201267
assert.deepStrictEqual({ desktopHeight, phoneHeight: phoneChatRow.style.height }, {
1221-
desktopHeight: '28px',
1222-
phoneHeight: '44px',
1268+
desktopHeight: '30px',
1269+
phoneHeight: '46px',
12231270
});
12241271
});
12251272

@@ -1991,8 +2038,8 @@ suite('Sessions - SessionsList', () => {
19912038

19922039
const row = targetRow();
19932040
assert.ok(row, 'target row should render after growing the viewport');
1994-
// Base chat rows are 28px; a reconciled approval must reserve more.
1995-
assert.ok(parseInt(row.style.height) > 28, `expected reconciled height to reserve the approval row, got ${row.style.height}`);
2041+
// Base chat rows reserve 30px including spacing; an approval must reserve more.
2042+
assert.ok(parseInt(row.style.height) > 30, `expected reconciled height to reserve the approval row, got ${row.style.height}`);
19962043
assert.ok(row.querySelector('.session-approval-row.visible'), 'approval row should be visible on the re-rendered target');
19972044
});
19982045
});

0 commit comments

Comments
 (0)