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.

Sign up for GitHub

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

Jump to bottom

theming: fix decoration color theming #7330

Merged
merged 1 commit into from
Apr 27, 2020
Merged

theming: fix decoration color theming #7330

merged 1 commit into from
Apr 27, 2020

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Mar 12, 2020 •
edited
Loading

What it does

Fixes #6839

The following commit addresses the following problems:

  • fixes the DecorationOptions color to also accept the id of color variables.
  • fixes the incorrect application of decorations for scm
    (includes additions, deletions, and modifications).
  • fixes the color variables for editorOverviewRuler, aligning the values with those of vscode.
  • aligns the fix to other areas of the framework, notably search-in-workspace.

How to test

  1. start the application with a workspace under version control (git).
  2. verify that additions, modifications, deletions are properly themed (similarly to vscode).
  3. change themes, and verify that colors are properly applied (ex: custom git-neon).
  4. perform a search-in-workspace search, verify that results are properly themed.
Screen Shot 2020-04-24 at 10 46 45 AM

Review checklist

Reminder for reviewers

Signed-off-by: Vincent Fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto vince-fugnitto added theming issues related to theming scm issues related to the source control manager labels Mar 12, 2020
@vince-fugnitto vince-fugnitto self-assigned this Mar 12, 2020
Anasshahidd21
Anasshahidd21 approved these changes Mar 12, 2020
View reviewed changes
Copy link
Contributor

@Anasshahidd21 Anasshahidd21 left a comment

Choose a reason for hiding this comment

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

LGTM. Tested all three options, addition, deletion, and modification.
The colors are displayed as required on the minimap as well as on the code-editor.

@vince-fugnitto
theming: fix decoration color theming
f8e04d7 
Fixes #6839

The following commit fixes the following problems:
- fixes the `DecorationOptions#color` to accept `ids` of color variables.
- fixes the decorations provided by `scm` (additions, modifications, removals).
- fixes the color variables for `scm`, aligning the values with vscode.
- fixes the decorations provided by the `siw` (search-in-workspace).

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
akosyakov
akosyakov approved these changes Apr 24, 2020
View reviewed changes
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

code-wise it looks good to me, but someone has to test

lmcbout
lmcbout approved these changes Apr 24, 2020
View reviewed changes
Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

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

LGTM
Tested with differnts theme colors for SCM view, search workspace, editor minimap and it looks ok.
Gook work.

@vince-fugnitto vince-fugnitto changed the title scm: fix theming colors for additions, modifications, deletions theming: fix decoration color theming ('scm', 'siw') Apr 24, 2020
@vince-fugnitto vince-fugnitto changed the title theming: fix decoration color theming ('scm', 'siw') theming: fix decoration color theming Apr 24, 2020
@vince-fugnitto vince-fugnitto added the search in workspace issues related to the search-in-workspace label Apr 24, 2020
Anasshahidd21
Anasshahidd21 approved these changes Apr 27, 2020
View reviewed changes
Copy link
Contributor

@Anasshahidd21 Anasshahidd21 left a comment

Choose a reason for hiding this comment

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

Works fine on my side.
Tested addition,deletion, modification on different themes along with a custom theme.

@vince-fugnitto vince-fugnitto merged commit ea56951 into master Apr 27, 2020
@vince-fugnitto vince-fugnitto deleted the vf/GH-6839 branch April 27, 2020 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Anasshahidd21 Anasshahidd21 Anasshahidd21 approved these changes

@akosyakov akosyakov akosyakov approved these changes

@lmcbout lmcbout lmcbout approved these changes

@AlexTugarev AlexTugarev Awaiting requested review from AlexTugarev

@elaihau elaihau Awaiting requested review from elaihau

Assignees

@vince-fugnitto vince-fugnitto

Labels
scm issues related to the source control manager search in workspace issues related to the search-in-workspace theming issues related to theming
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[theming] minimap is incorrectly decorated for scm updates (additions, modifications)
4 participants
@vince-fugnitto @lmcbout @akosyakov @Anasshahidd21