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

Let me theme diff editor's inserted text background differently in the scroll bar #66223

Closed
Tyriar opened this issue Jan 8, 2019 · 8 comments
Assignees
Labels
diff-editor Diff editor issues feature-request Request for new features or functionality themes Color theme issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jan 8, 2019

Currently diffEditor.insertedTextBackground and diffEditor.removedTextBackground are used to color both the background in the editor and the decorations in the scrollbar. The problem is in order to make the text contrast good enough so that I can read the text, that make the color in the scroll bar unreadable.

Before (good scroll bar, bad editor):

image

After (bad scroll bar, good editor):

image

Another way to fix this problem which is probably even better is to allow the background to only apply to the gutter and not the text, that way I can use a vibrant diff color and not worry about it clashing with every possible text color.

This is also a bit of an accessibility issue with the default theme:

image

@Tyriar Tyriar added feature-request Request for new features or functionality themes Color theme issues diff-editor Diff editor issues labels Jan 8, 2019
@Tyriar
Copy link
Member Author

Tyriar commented Jan 8, 2019

This is what I'd like to do:

image

So I guess 2-4 more theme keys:

diffEditor.insertedTextGutter
diffEditor.removedTextGutter

Maybe:

diffEditor.insertedTextScroll
diffEditor.removedTextScroll

Provided that the scroll color is not purely based on the background color of the editor by design?

I can try do this if I get some guidance on the approach and a code pointer.

@usernamehw
Copy link
Contributor

usernamehw commented Jan 8, 2019

Could there be added another feature?

Currently colors diffEditor.insertedTextBackground & diffEditor.removedTextBackground are used to style:

  1. Background of the entire changed line
  2. Background of the piece of actually changed code in line (derived / 2x less transparent?)

That results - if I change color to be too transparent - the changed code is barely visible.
If I change color to be too much opaque - the changed entire lines (where the entire line highlighted is barely readable.

Is it possible to split this in 2 themable colors:

  1. One color (very very transparent) to denote changed the entire line
  2. Second color (less transparent) to denote changed code in line

Maybe it could work well with that that gutter color change...

@Yanpas
Copy link
Contributor

Yanpas commented May 30, 2019

I also vote for custom theme colors for problems in the scroll bar (info, warning, error)

image
image

@joaomoreno joaomoreno removed their assignment Jun 3, 2019
@dcastil
Copy link

dcastil commented Jun 13, 2019

I find the text highlighting really distracting quite often and would like to be able to replace it with gutters instead. This would be especially helpful in the diff view.

@JonathanTroyer
Copy link

image

Just to reiterate @usernamehw's comment, this is what diffs look like in Material Theme (one of the most popular themes for VSCode). It looks like people are complaining there about this too.

@luketanner
Copy link

Is it possible to split this in 2 themable colors:

1. One color (very very transparent) to denote changed the entire line

2. Second color (less transparent) to denote changed code in line

Do you know if there's been any progress on this? Or does anyone know of any workarounds?

The lack of contrast between the background of the changed code within the line and the background of the entire line itself is extremely difficult for my eyes to distinguish.

Compare, for example, against GitHub's diff view (for PRs etc.), which has far stronger contrast and is much more readable.

@sisrfeng

This comment has been minimized.

@alexdima alexdima modified the milestones: Backlog, October 2021 Oct 18, 2021
@alexr00 alexr00 modified the milestones: October 2021, November 2021 Oct 29, 2021
@alexdima alexdima modified the milestones: November 2021, December 2021 Dec 1, 2021
@alexdima alexdima modified the milestones: January 2022, February 2022 Jan 27, 2022
alexdima added a commit that referenced this issue Feb 18, 2022
@alexdima
Copy link
Member

alexdima commented Feb 18, 2022

Together with changes for #103207 , I've added 6 new colors. e.g.

	"workbench.colorCustomizations": {
		"diffEditor.insertedTextBackground": "#00ff007c",
		"diffEditor.removedTextBackground": "#ff00007c",

		"diffEditor.insertedLineBackground": "#22336866",
		"diffEditor.removedLineBackground": "#72336a66",

		"diffEditorGutter.insertedLineBackground": "#223368ff",
		"diffEditorGutter.removedLineBackground": "#72336aff",

		"diffEditorOverview.insertedForeground": "#02b40b",
		"diffEditorOverview.removedForeground": "#a10000"
	},

image

cc @aeschli for feedback on color names. I've added new "namespaces" diffEditorGutter and diffEditorOverview

alexdima added a commit that referenced this issue Feb 18, 2022
alexdima added a commit that referenced this issue Feb 18, 2022
@alexdima alexdima added the verification-needed Verification of issue is requested label Feb 22, 2022
@Tyriar Tyriar added the verified Verification succeeded label Feb 22, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
diff-editor Diff editor issues feature-request Request for new features or functionality themes Color theme issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

10 participants