Bug 497984 - Stop using COLOR_INFO constants in PopupDialog
Summary: Stop using COLOR_INFO constants in PopupDialog
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.6   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.6.2   Edit
Assignee: Leo Ufimtsev CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy, ui
Depends on:
Blocks:
 
Reported: 2016-07-15 11:28 EDT by Alexander Kurtakov CLA
Modified: 2017-03-01 09:48 EST (History)
7 users (show)

See Also:
daniel_megert: review+


Attachments
Inconsistent quick outline dialog. (339.88 KB, image/png)
2016-07-15 11:34 EDT, Alexander Kurtakov CLA
no flags Details
Javadoc popup still looks misplaced (35.24 KB, image/png)
2016-08-19 05:06 EDT, Lars Vogel CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kurtakov CLA 2016-07-15 11:28:07 EDT
COLOR_INFO constants are used for tooltips drawing (as stated in the javadoc even). As dialogs are supposed to be user interacting it is best to switch to COLOR_LIST_[BACKGROUND|FOREGROUND] as selection colors are also based on the same widget thus inconsistency like white on white, blue on blue by default or when selected will be prevented.
Further more this would give a bit more predictable dialog colors in eclipse as some system themes use funky yellow, blue, black tooltip colors (aka COLOR_INFO_*) making these dialogs not integrate well with the workbench.
Comment 1 Alexander Kurtakov CLA 2016-07-15 11:34:49 EDT
Created attachment 263130 [details]
Inconsistent quick outline dialog.
Comment 2 Eclipse Genie CLA 2016-07-15 11:37:03 EDT
New Gerrit change created: https://git.eclipse.org/r/77402
Comment 4 Lars Vogel CLA 2016-08-19 04:44:27 EDT
Can we downport this to 4.6.1?
Comment 5 Dani Megert CLA 2016-08-19 04:55:14 EDT
(In reply to Lars Vogel from comment #4)
> Can we downport this to 4.6.1?

-1. The visual change is too big for that. I would at least wait for 4.6.2 or even 4.6.3 with that.
Comment 6 Eclipse Genie CLA 2016-08-19 05:04:17 EDT
New Gerrit change created: https://git.eclipse.org/r/79328
Comment 7 Lars Vogel CLA 2016-08-19 05:06:35 EDT
Created attachment 263668 [details]
Javadoc popup still looks misplaced

Unfortunately the Javadoc popup still looks bad. Not sure if it should be affected by this change.
Comment 8 Dani Megert CLA 2016-08-19 05:14:01 EDT
(In reply to Lars Vogel from comment #7)
> Created attachment 263668 [details]
> Javadoc popup still looks misplaced
> 
> Unfortunately the Javadoc popup still looks bad. Not sure if it should be
> affected by this change.

That's a different thing.
Comment 9 Sergey Prigogin CLA 2016-08-19 13:30:48 EDT
(In reply to Dani Megert from comment #8)
> That's a different thing.

Is there a separate bug for the Javadoc popup?
Comment 10 Eric Williams CLA 2016-08-19 13:55:19 EDT
(In reply to Sergey Prigogin from comment #9)
> (In reply to Dani Megert from comment #8)
> > That's a different thing.
> 
> Is there a separate bug for the Javadoc popup?

The Javadoc issue will be slightly alleviated by the fix for bug 477950. In general I think a fix for the Javadoc issue should be similar to this one: perhaps make the Javadoc popup use the same colors as the editors. 

The tooltip colors work well for things with a white foreground and a black background. This color scheme is ideal for short pieces of information. Once you start having large groups of text (not all white, some of it dark blue, etc.) it gets a bit difficult to read. It's also worth mentioning that the Javadoc popup isn't strictly a tooltip, it's a Browser widget.
Comment 11 Sergey Prigogin CLA 2016-08-19 14:06:10 EDT
(In reply to Eric Williams from comment #10)

Javadoc popup should probably use the same colors as the Javadoc view.
Comment 12 Eric Williams CLA 2016-08-19 14:11:00 EDT
(In reply to Sergey Prigogin from comment #11)
> (In reply to Eric Williams from comment #10)
> 
> Javadoc popup should probably use the same colors as the Javadoc view.

Doesn't the Javadoc view use the same COLOR_INFO_* colors as the popup?
Comment 13 Sergey Prigogin CLA 2016-08-19 14:27:19 EDT
(In reply to Eric Williams from comment #12)

You're right, it does use COLOR_INFO_* colors, which means that it is affected by this issue together with the Javadoc popup.
Comment 14 Eric Williams CLA 2016-08-19 14:45:24 EDT
(In reply to Sergey Prigogin from comment #13)
> You're right, it does use COLOR_INFO_* colors, which means that it is
> affected by this issue together with the Javadoc popup.

In this case, I think the Javadoc colors should be standard for all Javadoc widgets: the popups, Javadocs view, etc. If the Javadoc popups adopt the same color scheme as the editor then the Javadoc view should as well.
Comment 16 Eclipse Genie CLA 2016-08-24 07:48:05 EDT
New Gerrit change created: https://git.eclipse.org/r/79609
Comment 17 Lars Vogel CLA 2016-08-24 08:24:16 EDT
(In reply to Sergey Prigogin from comment #9)
> (In reply to Dani Megert from comment #8)
> > That's a different thing.
> 
> Is there a separate bug for the Javadoc popup?

Is was unable to find one and created Bug 500196
Comment 18 Lars Vogel CLA 2016-09-20 04:25:28 EDT
(In reply to Eclipse Genie from comment #2)
> New Gerrit change created: https://git.eclipse.org/r/77402

4.6.1 is finished, Alex can you prepare the downport Gerrit?
Comment 19 Alexander Kurtakov CLA 2016-09-20 05:15:06 EDT
I would appreciate someone taking it as I would not be able to look into it in the next week or even 2.
Comment 20 Leo Ufimtsev CLA 2016-09-20 10:38:33 EDT
(In reply to Alexander Kurtakov from comment #19)
> I would appreciate someone taking it as I would not be able to look into it
> in the next week or even 2.

Working on it.
Comment 21 Eclipse Genie CLA 2016-09-20 12:49:42 EDT
New Gerrit change created: https://git.eclipse.org/r/81499
Comment 22 Leo Ufimtsev CLA 2016-09-20 12:51:21 EDT
(In reply to Eclipse Genie from comment #21)
> New Gerrit change created: https://git.eclipse.org/r/81499

I don't have commit rights to platform.ui. Could someone please review/merge patch into R6_4?

Thanks.