Skip to content

Conversation

@gjsjohnmurray
Copy link
Contributor

This PR fixes #120429

To test, verify that all the places where debug codicons previously displayed in color still do so.

@gjsjohnmurray
Copy link
Contributor Author

/assign @isidorn

@gjsjohnmurray
Copy link
Contributor Author

gjsjohnmurray commented Apr 5, 2021

To see the benefit of this change amend this line of statusbar-sample:

https://github.com/microsoft/vscode-extension-samples/blob/63d86e3a0e459b2adf26487204891f7af62898ce/statusbar-sample/src/extension.ts#L37

to append $(debug-start) $(debug-pause) $(debug-stop) $(debug-disconnect) $(debug-restart) $(debug-step-over) $(debug-step-into) $(debug-step-out) $(debug-continue) $(debug-step-back) to text.

Build and run the extension. Open a file or create a new one with at least 2 lines. Select at least 2 lines. View the statusbar item.

Before this PR:

image

After this PR:

image

@isidorn
Copy link
Collaborator

isidorn commented Apr 6, 2021

@gjsjohnmurray thanks for creating this PR, however the problem is the following, we use those icons in 3 places:

  1. Debug toolbar floating
  2. Debug toolabar in the debug viewlet title area (set "debug.toolbarLocation": "docked")
  3. Inline callstack actions. Have mutliple sessions debugging and hover over the debug session item in the CALL STACK view

Your PR correctly handles 1, but breaks 2 and 3 if I am not mistaken.

@gjsjohnmurray
Copy link
Contributor Author

1 and 3 both work correctly:

image

You are correct that I wasn't handling 2 (docked toolbar)

image

Which class do you suggest I allow the colors for? Maybe monaco-toolbar ?

image

@miguelsolorio
Copy link
Contributor

An alternative, if this is specifically an issue with status bar items, is to set color: inherit !important on the status bar items:

.monaco-workbench .part.statusbar > .items-container > .statusbar-item span.codicon {
text-align: center;
font-size: 14px;
color: inherit;
}

@gjsjohnmurray
Copy link
Contributor Author

An alternative, if this is specifically an issue with status bar items, is to set color: inherit !important on the status bar items:

@misolori the original issue was that the colors that get added to these debug codicons mean they don't look good when used on the status bar. My initial suggestion had used a :not to prevent this, but you asked for colors to be permitted only where the debug part of VS Code uses them.

I'm not clear what effect your proposed color: inherit !important would have. Maybe I should just try it and see.

@isidorn
Copy link
Collaborator

isidorn commented Apr 7, 2021

Yeah I also suggest that we try to have a simpler approach.
I am not a fan of using important though.
I am also not 100% convinced we need to fix this asap. So if we get a nice solution I will happily merge if not no problem.

@gjsjohnmurray
Copy link
Contributor Author

I am also not 100% convinced we need to fix this asap.

Maybe not asap, but the longer it remains unchanged the more places these colored codicons will start appearing in the UI (e.g. inline menu contributions to view/item/context, so if we want the rule to be that they only get colored when used in core debug UI I think we should apply the constraint sooner rather than later.

If we are content to let them get colored wherever they appear I still think we should prevent it happening on the status bar. My initial proposal did that by using :not

Or can we add -uncolored aliases to the codicons here and here so extension devs can choose to use them via names that won't trigger the CSS coloring?

@weinand weinand added this to the August 2021 milestone Aug 4, 2021
@isidorn
Copy link
Collaborator

isidorn commented Aug 10, 2021

This is a debt color issue which I would leave up to @misolori to tackle when he thinks is best.
For now unassigning myself and moving out of current milestone.

@isidorn isidorn removed their assignment Aug 10, 2021
@isidorn isidorn removed this from the August 2021 milestone Aug 10, 2021
@miguelsolorio
Copy link
Contributor

Closing as this was fixed, see #120429 (comment)

@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2021
@gjsjohnmurray gjsjohnmurray deleted the fix-120429 branch August 16, 2024 04:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Debug codicons are colored with a CSS selector that's excessively broad

5 participants