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

modify the feature of quick input, when overflowed the elipsses shows… #145450

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kipackjeong
Copy link

@kipackjeong kipackjeong commented Mar 19, 2022

… on left

This PR fixes #143956

20220319_082728

@TylerLeonhardt
Copy link
Member

@kipackjeong can you attach a screenshot of the change?

Additionally, this isn't finished yet because we should make a new CSS class to do this behavior and then add it to the respective html tags that are created in the JS:

https://github.com/microsoft/vscode/blob/main/src/vs/base/parts/quickinput/browser/quickInputList.ts#L113

Then once you have that working we can talk about adding a setting to vscode that will allow you to set this behavior.

@TylerLeonhardt TylerLeonhardt marked this pull request as draft March 19, 2022 15:37
@kipackjeong
Copy link
Author

kipackjeong commented Mar 19, 2022

@TylerLeonhardt
Thank you for the feedback and please forgive my shortcomings, this was my first time contributing to the open source project.
I have attached the screenshot of the changes. However, I do not understand why we have to make a new CSS class. I would appreciate in depth explanation on this please. Thank you!

@TylerLeonhardt
Copy link
Member

@kipackjeong no worries and by the way, Draft PRs are totally fine to open. I'm happy to help you along the journey, just be patient with me as I have a few things I am working on at the same time.

We wanted the behavior you have introduced to be behind a setting like search.quickOpen.overflowOnLeft which would control whether the ellipsis would be shown on the right or left of the line.

In order to do this, I suggest making a CSS class (or a few if that's what is needed) that we can programmatically add here:
https://github.com/microsoft/vscode/blob/main/src/vs/base/parts/quickinput/browser/quickInputList.ts#L113

based on if our new setting is set.

@kipackjeong kipackjeong reopened this Mar 19, 2022
@kipackjeong kipackjeong marked this pull request as ready for review March 19, 2022 21:07
@kipackjeong
Copy link
Author

kipackjeong commented Mar 19, 2022

@TylerLeonhardt I made changes respective to your comments, can you review this please. Later now I noticed that proper spelling is "ellipsis",but I will change that.

Created new CSS Class: .left-ellipses

Added on labelContainer, nameContainer, and descriptionContainer in both the css file and in dom:

  • label-container.left-ellipses
  • name-container.left-ellipses
  • description-container.left-ellipses

Added this style on those three elements :

.quick-input-list .monaco-icon-label-container.left-ellipses {
    display: flex;
    align-items: center;
}

.quick-input-list .monaco-icon-name-container.left-ellipses {
    display:inline-block;
    block-size: 1.5rem;
}
.quick-input-list .monaco-icon-description-container.left-ellipses {
    display:inline-block;
    block-size: 1.5rem;
    overflow: hidden;
    text-overflow: ellipsis;
    width: 100%;
    direction: rtl;
    text-align: left;
}

For dom manipulation, when data.label is created with IconLabel class Instance, created and added new option ellipsesOnLeft

/* quickInputList.ts */
        data.label = new IconLabel(row1, { supportHighlights: true, supportDescriptionHighlights: true, supportIcons: true, ellipsesOnLeft: true });

Then added ellipsesOnLeft logic inside of IconLabel contsructor

/* iconLabel.ts */

if (options?.ellipsesOnLeft) {

            this.labelContainer.classList.add('left-ellipses');
            nameContainer.classList.add('left-ellipses');
            this.descriptionContainer.element.classList.add('left-ellipses');

        }

Now works well

When ellipsesOnLeft option is false
ksnip_20220319-170921
20220319_170123

When it is true

ksnip_20220319-170915

20220319_170211

@gjsjohnmurray
Copy link
Contributor

Maybe give the setting a less specific name and make it an enum with only two values at the moment, left and right. That leaves scope for future enhancement.

@kipackjeong
Copy link
Author

@gjsjohnmurray I think that is a good idea. For instance like Ellipsis.Left and Ellipsis.Right?

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Mar 21, 2022

@kipackjeong I'm thinking: EllipsisLocation with Left and Right and then hopefully in the near future we can figure out a safe and fast way to do ellipsis in the middle and we can have a Middle option (not required for this PR).

@TylerLeonhardt
Copy link
Member

Once you get Left and Right supported, you can make it a true VS Code setting by adding to the following:

'search.quickOpen.includeSymbols': {
type: 'boolean',
description: nls.localize('search.quickOpen.includeSymbols', "Whether to include results from a global symbol search in the file results for Quick Open."),
default: false
},
'search.quickOpen.includeHistory': {
type: 'boolean',
description: nls.localize('search.quickOpen.includeHistory', "Whether to include results from recently opened files in the file results for Quick Open."),
default: true
},
'search.quickOpen.history.filterSortOrder': {
'type': 'string',
'enum': ['default', 'recency'],
'default': 'default',
'enumDescriptions': [
nls.localize('filterSortOrder.default', 'History entries are sorted by relevance based on the filter value used. More relevant entries appear first.'),
nls.localize('filterSortOrder.recency', 'History entries are sorted by recency. More recently opened entries appear first.')
],
'description': nls.localize('filterSortOrder', "Controls sorting order of editor history in quick open when filtering.")
},

Then the plumbing fun begins...

follow the reference here:

includeSymbols: searchConfig?.quickOpen.includeSymbols,

alllllll the way down to the IconLabel.

I guess this issue was a bit more complicated than I thought... 😅

@kipackjeong
Copy link
Author

kipackjeong commented Apr 2, 2022

@TylerLeonhardt Thank you and sorry it has been over ten days of no reply, I was running so busy.
I added an option on
search.contribution.ts
and also added on

  'search.quickOpen.ellipsisLocation': {
	  type: 'string',
	  default: 'right',
	  enum: ['left', 'right'],
	  enumDescriptions: [
		  nls.localize('quickInput.ellipsisLocation.right', 'The ellipsis will show on the right side when the file path string overflows'),
		  nls.localize('quickInput.ellipsisLocation.left', 'The ellipsis will show on the left side when the file path string overflows')
	  ]
  
  },

anythingQuickAccess.ts

	private get configuration() {
		const editorConfig = this.configurationService.getValue<IWorkbenchEditorConfiguration>().workbench?.editor;
		const searchConfig = this.configurationService.getValue<IWorkbenchSearchConfiguration>().search;
		const quickAccessConfig = this.configurationService.getValue<IWorkbenchQuickAccessConfiguration>().workbench.quickOpen;

		return {
			openEditorPinned: !editorConfig?.enablePreviewFromQuickOpen || !editorConfig?.enablePreview,
			openSideBySideDirection: editorConfig?.openSideBySideDirection,
			includeSymbols: searchConfig?.quickOpen.includeSymbols,
			includeHistory: searchConfig?.quickOpen.includeHistory,
			ellipsisLocation: searchConfig?.quickOpen.ellipsisLocation,
			historyFilterSortOrder: searchConfig?.quickOpen.history.filterSortOrder,
			shortAutoSaveDelay: this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY,
			preserveInput: quickAccessConfig.preserveInput
		};
	}

But couldn't figure out how I can use this in InputLabel to manipulate the dom.

@kipackjeong
Copy link
Author

@TylerLeonhardt Thank you and sorry it has been over ten days of no reply, I was running so busy. I added an option on search.contribution.ts and also added on

  'search.quickOpen.ellipsisLocation': {
	  type: 'string',
	  default: 'right',
	  enum: ['left', 'right'],
	  enumDescriptions: [
		  nls.localize('quickInput.ellipsisLocation.right', 'The ellipsis will show on the right side when the file path string overflows'),
		  nls.localize('quickInput.ellipsisLocation.left', 'The ellipsis will show on the left side when the file path string overflows')
	  ]
  
  },

anythingQuickAccess.ts

	private get configuration() {
		const editorConfig = this.configurationService.getValue<IWorkbenchEditorConfiguration>().workbench?.editor;
		const searchConfig = this.configurationService.getValue<IWorkbenchSearchConfiguration>().search;
		const quickAccessConfig = this.configurationService.getValue<IWorkbenchQuickAccessConfiguration>().workbench.quickOpen;

		return {
			openEditorPinned: !editorConfig?.enablePreviewFromQuickOpen || !editorConfig?.enablePreview,
			openSideBySideDirection: editorConfig?.openSideBySideDirection,
			includeSymbols: searchConfig?.quickOpen.includeSymbols,
			includeHistory: searchConfig?.quickOpen.includeHistory,
			ellipsisLocation: searchConfig?.quickOpen.ellipsisLocation,
			historyFilterSortOrder: searchConfig?.quickOpen.history.filterSortOrder,
			shortAutoSaveDelay: this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY,
			preserveInput: quickAccessConfig.preserveInput
		};
	}

But couldn't figure out how I can use this in InputLabel to manipulate the dom.

@TylerLeonhardt Hello Tyler, any thoughts on it yet?

@LangLangBart
Copy link

Referencing ticket: #156062
Exactly what I was looking for.

The path is truncated from the left, like in macOS Spotlight. Tested this patch with the latest dev version and it works fine.

@starball5
Copy link

starball5 commented Jul 9, 2023

Note: in a kind of sick outcome, #156062 got closed as a dup of #58040, and both are closed and locked.

Related on Stack Overflow: How do I improve differentiation of long paths when searching for a file when there are others of the same name in VS Code's Quick Open feature?

@xzbdmw
Copy link

xzbdmw commented Jan 10, 2024

aha why no reply then

@TylerLeonhardt
Copy link
Member

Sorry for the delay, if you'd like to still continue, please handle the merge conflicts. I also would like to see a setting for this behavior so it's opt-in. Basically you will need to pass the value all the way down and add a CSS class to the InputBox depending on the setting value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better path presentation in quick open window
6 participants