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

Remove hard-coded zebra in running extensions Fixes #40432 #41496

Merged
merged 3 commits into from
Jan 16, 2018
Merged

Remove hard-coded zebra in running extensions Fixes #40432 #41496

merged 3 commits into from
Jan 16, 2018

Conversation

usernamehw
Copy link
Contributor

#40432

The same way it's already done in keybindings editor.

Also, there is another issue in "running extensions": .focused selector not overwriting .odd in dark and high contrast themes - it was broken before me.

@usernamehw
Copy link
Contributor Author

On the second thought... why it's even using typescript+css for striping and not :nth-child(odd)?

@usernamehw
Copy link
Contributor Author

And why it's using .even in the keybindings and .odd in running extensions?

@isidorn
Copy link
Contributor

isidorn commented Jan 12, 2018

@usernamehw all are fair suggestions feel free to add them to this PR and let me know when you are done so I review. Thanks

Focused even rows not using proper colors
Using css instead of typescript
Using css instead of typescript
@usernamehw
Copy link
Contributor Author

There was another issue: no hover on list items.

And another: click on list element usually applies "list.inactiveFocusBackground", but not in the "running extensions" list.


CSS nth-child(odd) is different from .odd.

.odd - index starting with 0
nth-child(odd) - index starting with 1

So it named now as even but behave exactly as it was before (in running extensions).

Changed from odd(as in CSS) to even in keybindings though... Because the first line merges with header.

Before:
before
After:
after

@isidorn

@usernamehw
Copy link
Contributor Author

Vendor prefixes from other browsers: are those legacy from monaco-editor or something else? You can search with this regex: ^\s+(-moz|-ms|-khtml|-o). Many of the -webkit ones are obsolete too.

@isidorn
Copy link
Contributor

isidorn commented Jan 15, 2018

@usernamehw thanks for your PR I have reveiwed the runtime extensions changes and this looks good to me 👍
As for the changes in the keybindings editor, assigning to @sandy081 to review. If he gives the green light we can merge this.
Just for future we prefer to have seperate PRs for seperate feature areas.
As for the monaco-editor we are still shipping the monaco-editor as a sepearte product and it supports other browsers, so that is why we keep other browsers in some areas.

@isidorn isidorn added this to the January 2018 milestone Jan 15, 2018
@sandy081
Copy link
Member

LGTM

@isidorn isidorn merged commit 7d34e22 into microsoft:master Jan 16, 2018
@isidorn
Copy link
Contributor

isidorn commented Jan 16, 2018

@usernamehw thanks a lot 🍻

@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.

None yet

3 participants