Skip to content

Conversation

JacksonKearl
Copy link
Contributor

@JacksonKearl JacksonKearl commented Aug 22, 2018

Closes #55796

Alternative implementation to #56511.

Main concern with this approach is the styling needed to separate the details from the list items. In this implementation I've added an additional color token that specifies the border colors for the dropdown and separating line, which defaults to the editorWidgetBorder color, and only applies if the existing settingsDropdownBorder is the same as the list background color. Its a confusing process that is required because of the lightness of the dropdown, and still doesnt give great results, as the editorWidgetBorder is typically used against darker backgrounds, and in as such appears washed out in some dark themes.

Dark+:
image

Dracula:
image

Monokai:
image

Solarized Dark:
image

Light themes are better because these tend to define a dropdown border color which acts as a good fallback:

Solarized Light:
image

@JacksonKearl JacksonKearl self-assigned this Aug 22, 2018
@JacksonKearl JacksonKearl added the settings-editor VS Code settings editor issues label Aug 22, 2018
@JacksonKearl JacksonKearl added this to the August 2018 milestone Aug 22, 2018
@@ -143,7 +144,7 @@ export function renderMarkdown(markdown: IMarkdownString, options: RenderOptions
}

if (options.actionHandler) {
options.actionHandler.disposeables.push(DOM.addStandardDisposableListener(element, 'click', event => {
options.actionHandler.disposeables.push(DOM.addStandardDisposableListener(element, options.eventToIntercept || 'click', event => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is sloppy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was needed because the widget disappears before a click can complete. Alternative ideas appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

If this was just to support links then I think we can remove it.

Copy link
Member

@roblourens roblourens left a comment

Choose a reason for hiding this comment

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

I think we want to avoid having links in enumDescriptions, one way or another - I can always fiddle with that later. LGTM!

@JacksonKearl JacksonKearl merged commit 346018a into microsoft:master Aug 24, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
settings-editor VS Code settings editor issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show setting enumDescriptions inside new dropdown control
2 participants