Draw completion proposals always as focused

[Christopher-Hermann]

Problem description

When opening the completion proposals via the keyboard, the focus will stay in the editor to be able to accept further user input. This is causing the completion proposal to be drawn in non focus colors. This colors can lead to UX problems, especially in dark theme. With this fix, the completion proposals are always drawn in focused colors.

Before the fix in dark theme, it was hard to sport the selected proposal:

With the fix, the proposal is colored in the focus color:

How to retest

1. Open a text editor and run the code completion (ctrl+space)
2. Check that the selected completion proposal is colored in the focused color

Fixes #1688

[Christopher-Hermann]

@iloveeclipse I had fixed this issue in the past, but we had to revert the changes since you found some issues on Linux: #1690
Could you please check if this more local fix is working on Linux?

On Mac and Windows it looks good.

[github-actions]

Test Results

 3 024 files  ±0   3 024 suites  ±0   2h 5m 38s ⏱️ - 11m 6s
 8 234 tests ±0   7 986 ✅ ±0  248 💤 ±0  0 ❌ ±0 
23 526 runs  ±0  22 735 ✅ ±0  791 💤 ±0  0 ❌ ±0 

Results for commit 81ff498. ± Comparison against base commit 89a19bf.

♻️ This comment has been updated with latest results.

[iloveeclipse]

I will test later, but in general I would assume this is not for this release, M3 is planned for tomorrow and the change affects everyone using content assist.

[iloveeclipse]

Could you please check if this more local fix is working on Linux?

On Mac and Windows it looks good.

I see errors on every popup close:

!ENTRY org.eclipse.ui 4 0 2025-02-12 16:51:52.599
!MESSAGE Unhandled event loop exception
!STACK 0
java.lang.NullPointerException: Cannot invoke "org.eclipse.swt.widgets.Widget.getDisplay()" because "event.item" is null
	at org.eclipse.jface.contentassist.CompletionProposalDrawSupport.handleEvent(CompletionProposalDrawSupport.java:54)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5862)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1652)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1678)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1657)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1396)
	at org.eclipse.swt.widgets.Control.release(Control.java:4753)
	at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:1576)
	at org.eclipse.swt.widgets.Canvas.releaseChildren(Canvas.java:279)
	at org.eclipse.swt.widgets.Decorations.releaseChildren(Decorations.java:503)
	at org.eclipse.swt.widgets.Shell.releaseChildren(Shell.java:3437)
	at org.eclipse.swt.widgets.Widget.release(Widget.java:1403)
	at org.eclipse.swt.widgets.Control.release(Control.java:4753)
	at org.eclipse.swt.widgets.Widget.dispose(Widget.java:576)
	at org.eclipse.swt.widgets.Shell.dispose(Shell.java:3354)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.hide(CompletionProposalPopup.java:1094)
	at org.eclipse.jface.text.contentassist.AsyncCompletionProposalPopup.hide(AsyncCompletionProposalPopup.java:359)
	at org.eclipse.jface.text.contentassist.ContentAssistant.hide(ContentAssistant.java:2268)
	at org.eclipse.jface.text.contentassist.ContentAssistant$Closer.mouseDown(ContentAssistant.java:221)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:230)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5862)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1652)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:5072)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4524)

[Christopher-Hermann]

I see errors on every popup close:

Yes I need to check this, there are also some "SWT Resource was not properly disposed" errors

[iloveeclipse]

there are also some "SWT Resource was not properly disposed" errors

These are most likely follow ups of original error

[Christopher-Hermann]

I see errors on every popup close:

Should be fixed.

Is the coloring working on Linux?

[Christopher-Hermann]

@iloveeclipse can you confirm that the coloring is working on linux like expected?

MacOS and Windows is working. Then I would merge this change.

[iloveeclipse]

Then I would merge this change.

You can't merge the change, we are in the RC phase.

[iloveeclipse]

can you confirm that the coloring is working on linux like expected?

I can't observe the original problem on Linux. The proposal table without this change looks exactly as with the change (blue selection on dark background).

[Christopher-Hermann]

If there are no objections I would merge the PR

[szarnekow]

If there are no objections I would merge the PR

Personally I find the focus color to distracting - it grabs too much of my attention while typing.

[Christopher-Hermann]

Personally I find the focus color to distracting - it grabs too much of my attention while typing.

Yes, good point. I will check if we can just change the non focus color to something more visible. The problem is, as far as I understood, that on Linux the code completion is always rendered with focus back ground color. So when changing the color, it will most probably change the behavior on Linux.

[Christopher-Hermann]

I tried using SWT.COLOR_TITLE_INACTIVE_BACKGROUND if the control is not focused, but this is even more worse (at least on MacOS).

So I see two ways to solve the issue:

1. Go with the approach as it is suggested in this PR: Just show the code completion as focus in any case
2. Introduce color definitions for table selections, as it was done in Highlighting problem when using the dark theme on Windows eclipse-platform/eclipse.platform.swt#811 #1690. Later on, when work on Highlighting problem when using the dark theme on Windows eclipse-platform/eclipse.platform.swt#811 #1690 is continued, we can reuse the already existing color definitions.

[fedejeanne]

@Christopher-Hermann I rebased on the current master and also added a final modifier.

I also set this one to a draft since I am trying to integrate it into #3659 via cherry-pick (hence the rebase). If I succeed then this PR could be dropped. I hope that's fine by you?

[Christopher-Hermann]

@Christopher-Hermann I rebased on the current master and also added a final modifier.

I also set this one to a draft since I am trying to integrate it into #3659 via cherry-pick (hence the rebase). If I succeed then this PR could be dropped. I hope that's fine by you?

Yes, this could be done with the new PR. I also tried this out and it works fine.

I guess the only thing is the decision/discussion if we want to color the completion always as selected, or if we should keep the non focus color. I guess for the second we don't have to do anything, this should work out of the box with #3659

[fedejeanne]

I guess the only thing is the decision/discussion if we want to color the completion always as selected, or if we should keep the non focus color.

For me it makes sense to use the same color that lists/trees have when their items are selected. I'll try that and see how it looks.