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.

Sign up for GitHub

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

Jump to bottom

Hide ToC during search if requested #194821

Merged
merged 6 commits into from
Oct 10, 2023
Merged

Hide ToC during search if requested #194821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
30b9a9a
Hide ToC during search if requested
rzhao271 Oct 4, 2023
e71dd63
Add smoke test
rzhao271 Oct 4, 2023
3f20923
Comment out lines to debug smoke test
rzhao271 Oct 4, 2023
70086df
Apply PR feedback
rzhao271 Oct 4, 2023
b9ab7ed
Fix smoke test
rzhao271 Oct 4, 2023
f8a056c
Colour sash when visible
rzhao271 Oct 10, 2023
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
3 changes: 0 additions & 3 deletions src/vs/workbench/contrib/preferences/browser/media/settingsEditor2.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@
text-decoration: underline;
}

.settings-editor.no-toc-search > .settings-body .settings-tree-container .monaco-list-row .monaco-tl-contents,
.settings-editor.narrow-width > .settings-body .settings-tree-container .monaco-list-row .monaco-tl-contents {
padding-left: 33px;
}
Expand Down Expand Up @@ -248,7 +247,6 @@
pointer-events: initial;
}

.settings-editor.no-toc-search > .settings-body .settings-toc-container,
.settings-editor.narrow-width > .settings-body .settings-toc-container {
display: none;
}
Expand Down Expand Up @@ -308,7 +306,6 @@
margin: auto;
}

.settings-editor.no-toc-search > .settings-body .settings-tree-container,
.settings-editor.narrow-width > .settings-body .settings-tree-container {
margin-left: 0px;
}
Expand Down
67 changes: 42 additions & 25 deletions src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ interface IFocusEventFromScroll extends KeyboardEvent {
}

const searchBoxLabel = localize('SearchSettings.AriaLabel', "Search settings");
const SEARCH_TOC_BEHAVIOR_KEY = 'workbench.settings.settingsSearchTocBehavior';

const SETTINGS_EDITOR_STATE_KEY = 'settingsEditorState';
export class SettingsEditor2 extends EditorPane {
Expand Down Expand Up @@ -1181,13 +1182,6 @@ export class SettingsEditor2 extends EditorPane {
this.telemetryService.publicLog2<SettingsEditorModifiedSettingEvent, SettingsEditorModifiedSettingClassification>('settingsEditor.settingModified', data);
}

private onSearchModeToggled(): void {
this.rootElement.classList.remove('no-toc-search');
if (this.configurationService.getValue('workbench.settings.settingsSearchTocBehavior') === 'hide') {
this.rootElement.classList.toggle('no-toc-search', !!this.searchResultModel);
}
}

private scheduleRefresh(element: HTMLElement, key = ''): void {
if (key && this.scheduledRefreshes.has(key)) {
return;
Expand Down Expand Up @@ -1518,7 +1512,27 @@ export class SettingsEditor2 extends EditorPane {
return match && match[1];
}

private triggerSearch(query: string): Promise<void> {
/**
* Toggles the visibility of the Settings editor table of contents during a search
* depending on the behavior.
*/
private toggleTocBySearchBehaviorType() {
const tocBehavior = this.configurationService.getValue<'filter' | 'hide'>(SEARCH_TOC_BEHAVIOR_KEY);
const hideToc = tocBehavior === 'hide';
if (hideToc) {
this.splitView.setViewVisible(0, false);
this.splitView.style({
separatorBorder: Color.transparent
});
} else {
this.splitView.setViewVisible(0, true);
rzhao271 marked this conversation as resolved.
Show resolved Hide resolved
this.splitView.style({
separatorBorder: this.theme.getColor(settingsSashBorder)!
});
}
}

private async triggerSearch(query: string): Promise<void> {
this.viewState.tagFilters = new Set<string>();
this.viewState.extensionFilters = new Set<string>();
this.viewState.featureFilters = new Set<string>();
Expand All @@ -1538,7 +1552,8 @@ export class SettingsEditor2 extends EditorPane {

if (query && query !== '@') {
query = this.parseSettingFromJSON(query) || query;
return this.triggerFilterPreferences(query);
await this.triggerFilterPreferences(query);
this.toggleTocBySearchBehaviorType();
} else {
if (this.viewState.tagFilters.size || this.viewState.extensionFilters.size || this.viewState.featureFilters.size || this.viewState.idFilters.size || this.viewState.languageFilter) {
this.searchResultModel = this.createFilterModel();
Expand All @@ -1557,7 +1572,6 @@ export class SettingsEditor2 extends EditorPane {
this.tocTree.setFocus([]);
this.viewState.filterToCategory = undefined;
this.tocTreeModel.currentSearchModel = this.searchResultModel;
this.onSearchModeToggled();

if (this.searchResultModel) {
// Added a filter model
Expand All @@ -1566,16 +1580,17 @@ export class SettingsEditor2 extends EditorPane {
this.refreshTOCTree();
this.renderResultCountMessages();
this.refreshTree();
this.toggleTocBySearchBehaviorType();
} else if (!this.tocTreeDisposed) {
// Leaving search mode
this.tocTree.collapseAll();
this.refreshTOCTree();
this.renderResultCountMessages();
this.refreshTree();
// Always show the ToC when leaving search mode
this.splitView.setViewVisible(0, true);
}
}

return Promise.resolve();
}

/**
Expand All @@ -1596,7 +1611,6 @@ export class SettingsEditor2 extends EditorPane {
}

filterModel.setResult(0, fullResult);

return filterModel;
}

Expand Down Expand Up @@ -1691,7 +1705,6 @@ export class SettingsEditor2 extends EditorPane {
// to make sure the search results count is set.
this.searchResultModel.setResult(type, result);
this.tocTreeModel.currentSearchModel = this.searchResultModel;
this.onSearchModeToggled();
} else {
this.searchResultModel.setResult(type, result);
this.tocTreeModel.update();
Expand Down Expand Up @@ -1789,18 +1802,22 @@ export class SettingsEditor2 extends EditorPane {
// opens for the first time.
this.splitView.layout(this.bodyContainer.clientWidth, listHeight);

const firstViewWasVisible = this.splitView.isViewVisible(0);
const firstViewVisible = this.bodyContainer.clientWidth >= SettingsEditor2.NARROW_TOTAL_WIDTH;

this.splitView.setViewVisible(0, firstViewVisible);
// If the first view is again visible, and we have enough space, immediately set the
// editor to use the reset width rather than the cached min width
if (!firstViewWasVisible && firstViewVisible && this.bodyContainer.clientWidth >= SettingsEditor2.EDITOR_MIN_WIDTH + SettingsEditor2.TOC_RESET_WIDTH) {
this.splitView.resizeView(0, SettingsEditor2.TOC_RESET_WIDTH);
const tocBehavior = this.configurationService.getValue<'filter' | 'hide'>(SEARCH_TOC_BEHAVIOR_KEY);
const hideTocForSearch = tocBehavior === 'hide' && this.searchResultModel;
if (!hideTocForSearch) {
const firstViewWasVisible = this.splitView.isViewVisible(0);
const firstViewVisible = this.bodyContainer.clientWidth >= SettingsEditor2.NARROW_TOTAL_WIDTH;

this.splitView.setViewVisible(0, firstViewVisible);
// If the first view is again visible, and we have enough space, immediately set the
// editor to use the reset width rather than the cached min width
if (!firstViewWasVisible && firstViewVisible && this.bodyContainer.clientWidth >= SettingsEditor2.EDITOR_MIN_WIDTH + SettingsEditor2.TOC_RESET_WIDTH) {
this.splitView.resizeView(0, SettingsEditor2.TOC_RESET_WIDTH);
}
this.splitView.style({
separatorBorder: firstViewVisible ? this.theme.getColor(settingsSashBorder)! : Color.transparent
});
}
this.splitView.style({
separatorBorder: firstViewVisible ? this.theme.getColor(settingsSashBorder)! : Color.transparent
});
}

protected override saveState(): void {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/preferences/common/preferencesContribution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ registry.registerConfiguration({
nls.localize('settingsSearchTocBehavior.hide', "Hide the Table of Contents while searching. The search results will not be grouped by category, and instead will be sorted by similarity to the query, with exact keyword matches coming first."),
nls.localize('settingsSearchTocBehavior.filter', "Filter the Table of Contents to just categories that have matching settings. Clicking a category will filter the results to that category. The search results will be grouped by category."),
],
'description': nls.localize('settingsSearchTocBehavior', "Controls the behavior of the settings editor Table of Contents while searching."),
'description': nls.localize('settingsSearchTocBehavior', "Controls the behavior of the Settings editor Table of Contents while searching. If this setting is being changed in the Settings editor, the setting will take effect after the search query is modified."),
rzhao271 marked this conversation as resolved.
Show resolved Hide resolved
'default': 'filter',
'scope': ConfigurationScope.WINDOW
},
Expand Down
21 changes: 21 additions & 0 deletions test/smoke/src/areas/preferences/preferences.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,33 @@ export function setup(logger: Logger) {
await app.code.dispatchKeybinding('enter');
await app.code.waitForElements('.line-numbers', false, elements => !!elements.length);

// Turn off line numbers
await app.workbench.settingsEditor.searchSettingsUI('editor.lineNumbers');
await app.code.waitAndClick('.settings-editor .monaco-list-rows .setting-item-control select', 2, 2);
await app.code.waitAndClick('.context-view .option-text', 2, 2);

await app.workbench.editors.selectTab('Untitled-1');
await app.code.waitForElements('.line-numbers', false, elements => !elements || elements.length === 0);
});

it('hides the toc when searching depending on the search behavior', async function () {
const app = this.app as Application;

// Hide ToC when searching
await app.workbench.settingsEditor.searchSettingsUI('workbench.settings.settingsSearchTocBehavior');
await app.code.waitAndClick('.settings-editor .monaco-list-rows .setting-item-control select', 2, 2);
await app.code.waitAndClick('.context-view .monaco-list-row:nth-child(1) .option-text', 2, 2);
await app.workbench.settingsEditor.searchSettingsUI('test');
await app.code.waitForElements('.settings-editor .settings-toc-container', false, elements => elements.length === 1 && elements[0].attributes['style'].includes('width: 0px'));
await app.code.waitForElements('.settings-editor .settings-body .monaco-sash', false, elements => elements.length === 1 && elements[0].className.includes('disabled'));

// Show ToC when searching
await app.workbench.settingsEditor.searchSettingsUI('workbench.settings.settingsSearchTocBehavior');
await app.code.waitAndClick('.settings-editor .monaco-list-rows .setting-item-control select', 2, 2);
await app.code.waitAndClick('.context-view .monaco-list-row:nth-child(2) .option-text', 2, 2);
await app.workbench.settingsEditor.searchSettingsUI('test');
await app.code.waitForElements('.settings-editor .settings-toc-container', false, elements => elements.length === 1 && !elements[0].attributes['style'].includes('width: 0px'));
await app.code.waitForElements('.settings-editor .settings-body .monaco-sash', false, elements => elements.length === 1 && !elements[0].className.includes('disabled'));
});
});
}