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

ToolItem with SWT.CHECK is not well displayed when checked and with background setted. #541

Closed
RoiSoleil opened this issue Jan 30, 2023 · 22 comments · Fixed by #542
Closed
Assignees
Labels
Windows Happens on Windows OS
Milestone

Comments

@RoiSoleil
Copy link
Contributor

RoiSoleil commented Jan 30, 2023

Describe the bug
ToolItem with SWT.CHECK is not well displayed when checked and with background setted.

ToolItem - Checked

ToolItem - Checked with background

With the fix :

ToolItem - Checked with background with fix

To Reproduce

Snippet153

Environment:

  1. Select the platform(s) on which the behavior is seen:
    • All OS
    • Windows
    • Linux
    • macOS
RoiSoleil pushed a commit to RoiSoleil/eclipse.platform.swt that referenced this issue Jan 30, 2023
RoiSoleil added a commit to RoiSoleil/eclipse.platform.swt that referenced this issue Jan 30, 2023
@Phillipus
Copy link
Contributor

Phillipus commented Jan 30, 2023

What problem is this trying to solve?

In your PR #542 you have removed this line on the Windows version of ToolBar:

result |= OS.TBCDRF_NOBACKGROUND;

As far as I can see, this affects the painting of all items, not just with SWT.CHECK, Also, the behaviour is different on Windows 11.

@RoiSoleil
Copy link
Contributor Author

I'm trying to solve the problem that when a ToolItem with SWT.CHECK is selected and has a background, the fact that it's selected is not displayed (as you can see in the screenshot 2 / my fix is screenshot 3). I will try to see for Linux and MacOS tonight.

It's will solve these issues : eclipse-platform/eclipse.platform.ui#467
eclipse-platform/eclipse.platform.ui#340
eclipse-platform/eclipse.platform.ui#530

@Phillipus
Copy link
Contributor

Sorry, but that's not clear to me when running your Snippet. On Windows 11 the Snippet behaves as expected.

@RoiSoleil
Copy link
Contributor Author

I will try on Windows 11 too.

@jasonhy-wang
Copy link
Contributor

Works as expected on Fedora 37

@lshanmug
Copy link
Member

@deepika-u Can you please try this out? The snippet is in the PR.

@RoiSoleil
Copy link
Contributor Author

In windows the bug is present when Eclipse is in darkmode. When in darkmode we dont we if a ToolItem is checked or not. This is really annoying. See an example without my fix :

image

And with my fix :

image

@Phillipus Are you sure that in Windows 11 it's ok ?

@Phillipus
Copy link
Contributor

As I said, this change might affect all toolbar items not just checked ones. It also affects the colour when hovering over any tool item. Is that expected?

RoiSoleil added a commit to RoiSoleil/eclipse.platform.swt that referenced this issue Feb 28, 2023
RoiSoleil added a commit to RoiSoleil/eclipse.platform.swt that referenced this issue Feb 28, 2023
@RoiSoleil
Copy link
Contributor Author

RoiSoleil commented Feb 28, 2023

As I said, this change might affect all toolbar items not just checked ones. It also affects the colour when hovering over any tool item. Is that expected?

Yes this is expected. Custom background doesn't meen hiding hovering toolitem.

@RoiSoleil
Copy link
Contributor Author

I tested on Windows 11 and i have the same problem in darkmode for ToomItem with check state.

Without my code :

Sans

With my code :

Avec

@Phillipus
Copy link
Contributor

Isn't the underlying problem something to do with theme rendering? See eclipse-platform/eclipse.platform.ui#467

@RoiSoleil
Copy link
Contributor Author

No for me it's the #26 that introduces a bug when dealing with custom background color on ToolItem. The checked state should not be hidden when a custom background is applied.
Please can someone deals with this bug because this is really annoying and there has been 4 releases with this bug.

@nnemkin
@vogella

@Phillipus
Copy link
Contributor

@RoiSoleil But your solution is not the right way to fix this, I think. As I said, this affects the painting of all items, not just with SWT.CHECK. Even your screenshot in #541 (comment) shows in incorrect blue background color for dark theme. Have you considered how you change might have side effects for all users?

@RoiSoleil
Copy link
Contributor Author

Please can you show me what's wrong in my screenshot ?
Also can you show me a correct way to show checked state for ToolItem using dark theme ?

@Phillipus
Copy link
Contributor

@RoiSoleil OK, I've looked more into this now. What I mean is that in the dark theme I would have expected the enabled state on Windows to use dark grey colors and an outline as on Mac:

toolbar

But I can also see that you have correctly identified #26 as the point where things regressed.

So, whereas your PR does put things back on Windows to how they were before that change, I wonder if the changes made to Toolbar in that commit need tweaking to arrive at a consistent solution on all platforms?

@RoiSoleil
Copy link
Contributor Author

I'm OK with you, it should be consistent on all platforms. Unfortunately, i'm not skilled enough to do that on Mac and Linux ...
I thinks someone more skilled than me should deal with this bug. Maybe you can ?
I really think it should be fixed for the next release because as said before its really annoying ...

@RoiSoleil
Copy link
Contributor Author

As i said the problem is not only with checked ToolItem, it's with every ToolItem with a background color. Here is an example of the Left Arrow under Windows, you can see how the hovering is not well painted :

Without

With my code, you can see how the hovering is now well painted :

Fixed

@RoiSoleil
Copy link
Contributor Author

@Phillipus What do you think?

@Phillipus
Copy link
Contributor

@RoiSoleil I can see how your PR ensures that the behaviour on Windows seems to be as it was before #26 but that commit also changed ToolBar on Mac and Linux. So maybe this needs some input from other SWT committers to see if there is a fix that might cover all platforms if required. Also, I think we may need to check if #26 has affected toolbar and tool tool items not just in dark theme.

@RoiSoleil
Copy link
Contributor Author

@Phillipus May be we can fix Windows first and then the other platforms (MacOS seems to be ok with you previous screenshot). My commit is not specific for darktheme, it is for all themes setting a background to ToolItem.

@lshanmug
@deepika-u

What do you think ?

@niraj-modi niraj-modi added this to the 4.28 M2 milestone Apr 25, 2023
@niraj-modi
Copy link
Member

@deepika-u Please verify the fix in next Eclipse I-Build. Thanks!

@deepika-u
Copy link
Contributor

I have checked this in below build, it works fine. So it is verified.
Eclipse SDK
Version: 2023-06 (4.28)
Build id: I20230426-1800
OS: Windows 11, v.10.0, x86_64 / win32
Java vendor: Eclipse Adoptium
Java runtime version: 20+36
Java version: 20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Windows Happens on Windows OS
Projects
None yet
6 participants