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

Reverting the bg color for StyledText, Button, Table controls #2181

Merged

Conversation

mvm-sap
Copy link
Contributor

@mvm-sap mvm-sap commented Aug 12, 2024

Reverting the changes done to bg colors of controls like StyledText, Button, Table in views.
Selected tab in TabbedPropertyList had grey background in view, even that has been changed to white with this change.
Issue #2173 has been addressed with this change.
Please again test and provide feedback if there are still some issues.

Below are the screenshots

Before:
image

After:
image

Reverting the changes done to bg colors of controls like StyledText,
Button, Table only in views to default from grey.
Selected tab in TabbedPropertyList had grey background in view, even
that has been changed to white in this change.

This change also includes Tool Item color change in linux and mac which
was missing in the previous PR 2168
@BeckerWdf
Copy link
Contributor

@Phillipus: Can you pls. review and test?

Copy link
Contributor

Test Results

 1 815 files  +  605   1 815 suites  +605   1h 33m 50s ⏱️ + 27m 52s
 7 688 tests ±    0   7 460 ✅ +    3  228 💤  -   3  0 ❌ ±0 
24 225 runs  +8 075  23 476 ✅ +7 839  749 💤 +236  0 ❌ ±0 

Results for commit 403317f. ± Comparison against base commit 26102ed.

@Phillipus
Copy link
Contributor

Thanks for the PR. I tested this with our RCP app and can confirm that the StyledText, Button controls in views and the selected tab in TabbedPropertyList are white in the light theme and the Table control has alternate row colors on Mac.

One slight issue on Mac is the background in a Label in a Group control:

Screenshot 2024-08-12 at 15 24 16

However, the same problem occurs in the current light theme on Mac.

@BeckerWdf
Copy link
Contributor

However, the same problem occurs in the current light theme on Mac.

So this has to be handled separately..

@Phillipus
Copy link
Contributor

So this has to be handled separately..

The following on Mac seems to fix it:

.View Group Label {
    background-color: unset;
}

@tomaswolf
Copy link
Member

Just noticed: the gray unstaged/staged changes trees in that abapGit view (probably also in the eGit staging view?) also look like they were disabled. (Perhaps they are, since there are no changes...) Perhaps trees inside forms also should be styled with a white background. How does e.g. the PDE manifest editor look with that new styling?

@Phillipus
Copy link
Contributor

Phillipus commented Aug 12, 2024

How does e.g. the PDE manifest editor look with that new styling?

If it's an EditPart it's not affected. This change affects controls in ViewParts. But if those same form-based controls are in a ViewPart they're grey.

Trees in ViewParts are grey so if you depend on that to show disabled state of a Tree you're out of luck.

@mvm-sap
Copy link
Contributor Author

mvm-sap commented Aug 13, 2024

So this has to be handled separately..

The following on Mac seems to fix it:

.View Group Label {
    background-color: unset;
}

If we unset, it will affect other views, find below the screenshot. This is in windows, might aswell affect Mac
image

I feel its better to set it this way:

.View Group Label{
background-color: inherit;
}

What do you think?

@BeckerWdf
Copy link
Contributor

Just noticed: the gray unstaged/staged changes trees in that abapGit view (probably also in the eGit staging view?) also look like they were disabled. (Perhaps they are, since there are no changes...) Perhaps trees inside forms also should be styled with a white background. How does e.g. the PDE manifest editor look with that new styling?

With this change or before this change?

@mvm-sap
Copy link
Contributor Author

mvm-sap commented Aug 13, 2024

Just noticed: the gray unstaged/staged changes trees in that abapGit view (probably also in the eGit staging view?) also look like they were disabled. (Perhaps they are, since there are no changes...) Perhaps trees inside forms also should be styled with a white background. How does e.g. the PDE manifest editor look with that new styling?

Yes, Staged and Unstaged will have grey backgrounds, as we have set the bg for a tree as grey in views.

@BeckerWdf
Copy link
Contributor

Can we merge this change?

@BeckerWdf
Copy link
Contributor

and the Table control has alternate row colors on Mac.

I don't see that on my Mac. Where do you see that? Can you show a screenshot?

@BeckerWdf BeckerWdf added this to the 4.33 M3 milestone Aug 13, 2024
@Phillipus
Copy link
Contributor

I feel its better to set it this way:

background-color: inherit;
}

What do you think?

That doesn't work. As I said it's Mac only so only belongs in the Mac CSS file .

@Phillipus
Copy link
Contributor

and the Table control has alternate row colors on Mac.

I don't see that on my Mac. Where do you see that? Can you show a screenshot?

This PR reverts the grey table so the alternate row colors is expected and desired (as it used to be).

@BeckerWdf
Copy link
Contributor

If nobody objects I will merge this today.

@BeckerWdf
Copy link
Contributor

Thanks for the PR. I tested this with our RCP app and can confirm that the StyledText, Button controls in views and the selected tab in TabbedPropertyList are white in the light theme and the Table control has alternate row colors on Mac.

One slight issue on Mac is the background in a Label in a Group control:

Screenshot 2024-08-12 at 15 24 16 However, the same problem occurs in the current light theme on Mac.

So this existed already even before @mvm-sap's changes? If yes. Pls. open a separate issue. And if you already have a solution proposal pls. also send a PR for it.

@BeckerWdf BeckerWdf merged commit b02a33b into eclipse-platform:master Aug 13, 2024
16 checks passed
@mvm-sap mvm-sap deleted the Revert_StyledText_Table_Button_BG branch September 23, 2024 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants