Skip to content

Fix missing styling from various icon container locations#88157

Merged
joaomoreno merged 3 commits intomicrosoft:masterfrom
mjcrouch:styling-bug
Jan 15, 2020
Merged

Fix missing styling from various icon container locations#88157
joaomoreno merged 3 commits intomicrosoft:masterfrom
mjcrouch:styling-bug

Conversation

@mjcrouch
Copy link
Copy Markdown
Contributor

@mjcrouch mjcrouch commented Jan 6, 2020

This PR fixes fixes #87668 - in which strikethrough is missing from scm provider resources

Note that there are 7 more references in various css files to .monaco-icon-label-description-container - which no longer appears to exist in the code. I have not yet analyzed whether they should also be updated to the new classes. (see below)

@mjcrouch mjcrouch changed the title fix missing strikethrough styling for scm resource states. fix missing strikethrough styling for scm resource states Jan 6, 2020
@mjcrouch
Copy link
Copy Markdown
Contributor Author

mjcrouch commented Jan 6, 2020

I notice that the tests failed due to hygiene checks, but this seems to have been caused by the update to TS 3.8 about an hour ago 464ee36 and is not related to the PR...

@mjcrouch
Copy link
Copy Markdown
Contributor Author

mjcrouch commented Jan 6, 2020

Having looked through the other instances a bit more I believe they should all be updated as well - haven't managed to actually produce the right conditions yet though.

For example suggest.css implies deprecated items should have strike through, but functions don't appear to have the deprecated tag applied to them when I make them deprecated via the comments, and as such the deprecated class isn't applied. (Probably something I'm doing wrong).

A couple of them refer to previous minor styling issues in the tab header, I wasn't able to reproduce the actual original issues with the incorrect css - nevertheless by inspecting the elements, the current css is clearly not applied as intended

A number of styles were missing from various locations due to the changes to support multiple icon labels in 8bb358f

This change replaces all instances of the old class name .monaco-icon-label-description-container with the selector .monaco-icon-label-container > .monaco-icon-name-container
@msftclas
Copy link
Copy Markdown

msftclas commented Jan 7, 2020

CLA assistant check
All CLA requirements met.

@mjcrouch mjcrouch changed the title fix missing strikethrough styling for scm resource states Fix missing styling from various icon container locations Jan 7, 2020
@mjcrouch
Copy link
Copy Markdown
Contributor Author

mjcrouch commented Jan 7, 2020

I've updated the PR to update all the styles that had the incorrect class name.

I was able to prove that deprecated strikethrough was broken by modifying the sample completion-sample extension.

I did clearly see that the correct overflow / text-overflow styles were applied to tabs following my change - though I couldn't see any visible difference from when they did not have the correct styles.

Same for the quick open dialog and the custom view tree (e.g. the npm scripts view) - flex: 1 was not applied, but I could not see any visible difference when it was applied.

I haven't worked out how to produce an element with .remote-help-tree-node-item-icon

(I seem to be good at timing my submissions badly, as the tests failed again due to an unrelated issue)

@joaomoreno
Copy link
Copy Markdown
Member

Great PR! Thank you!

@joaomoreno joaomoreno added this to the January 2020 milestone Jan 15, 2020
@joaomoreno joaomoreno merged commit 465fc2b into microsoft:master Jan 15, 2020
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strikethrough not applied on source control view & deprecated items. Styles are missing from other locations

3 participants