scide: make tooltip text black on Linux #2762

nhthn · 2017-03-02T22:06:23Z

I have no clue what I did here but it seems to fix #1704.

bagong · 2017-03-02T22:49:16Z

Hmm, this would indicate that the Q_WS_X11 branch is not used in a Linux compile, and in fact stackoverflow show various topics (but nothing very clear) that Q_WS_* should be changed to Q_OS_* when switching from Qt4 to Qt5. I think that makes sense with when that error turned up. See for example here:

https://www.kdab.com/porting-from-qt-4-to-qt-5/

So rather than changing the Mac/Windows code to Linux code, it might be worth trying if Q_OS_LINUX does the job?

If that turns out to be correct, some stuff Q_WS_X11 headed stuff might be worth inspecting too. Are the shortcut-key problems in linux fixed?

nhthn · 2017-03-03T00:37:34Z

Aha, hopefully we'll manage to kill several birds with one stone here. Unfortunately, the code depends on QGtkStyle, which seems to no longer exist in Qt 5.

nhthn · 2017-03-03T01:48:34Z

I've force pushed a commit that removes the dependency on QGtkStyle and just hardcodes the colors on Linux.

bagong · 2017-03-03T07:36:12Z

Maybe wise:
https://bugzilla.redhat.com/show_bug.cgi?id=1386404
So what do we have here? Window-manager vs. OS, Qt-company shaking of competitors, distribution policy, Linux flavor differences, supporting styles and configurability vs hardcoding... Just a little preprocessor macro, and composers become engineers. We were hoping Qt would make things easier ;)
Nevertheless this looks really promising. I think it will be the favorite fix in 3.9!

bagong · 2017-03-03T09:20:16Z

Hah, it's the only spot where QGtKStyle is used in the entire app. That looks like a worthy sacrifice until we get a Qt crack...

But: UNIX includes MACOS. Is that intended (I checked, if I change the color in the Q_OS_UNIX branch, the color changes on the mac too)?

nhthn · 2017-03-04T02:37:21Z

OK, Q_OS_LINUX it is. This should be ready to merge after we've confirmed that the colors are correct on Linux, Mac, and Windows.

bagong · 2017-03-04T08:48:53Z

Really sorry for being nitpicking here, it would definitely be nice to get this out of the room. The only quick logic that seems safe here is UNIX but not (MACOS or netBSD). What is typically done actually is that things are defined for MACOS and Windows (together or separate) first, and then code for "the rest" (assuming that IOS is not an issue). I am not sure what LINUX covers (I used that in the beginning as a quick demonstrator, not as a final), and there are/used to be/could be quite a few unixes that are not covered by LINUX but could be covered by your "safe" code.

HAHA, for checking if the colors are right, we do not need to test. We'd have to use a different color to verify (and I did on OSX) ;) Btw, I'd like to see a unittest that catches the wrong color 😄

.

nhthn · 2017-03-04T18:14:12Z

No worries about nitpicking. ✔️

bagong

Built with red on MacOS and Windows and got black ;) Thanks. A Qt-crack could maybe do something more nifty, but for our manpower I think this is an adequate fix and solves a tiny but annoying and much complained about problem ;)

It also can serve as model to fix some other problems at places using Q_WS_X11 which seem to have been overlooked when switching from Qt4 to Qt5. Those must all be going wrong, but they don't complain in the build as the alternative code compiles (but likely creates runtime bugs). I bet a bottle of wine that the keyboard problems on Linux have to do with this. ;)

I'll be so bold to approve this ;)

patrickdupuis · 2017-03-05T04:49:30Z

I tested on my system and i get black. Should i be looking at text colour elsewhere?

nhthn · 2017-03-05T05:46:18Z

@patrickdupuis As I understand the issue, the problem is that the tooltip text color matches that of the window manager's text color, but the background color is fixed at pale yellow. Therefore, the text is only unreadable on systems where the native tooltips are light-on-dark.

bagong · 2017-03-05T12:23:57Z

@patrickdupuis What did you expect? It's hard-coded to black now...

patrickdupuis · 2017-03-06T02:35:43Z

Indeed it is. My system was never affected by this, and I can confirm that this change has no negative side effects for me.

Let's merge and make our Ubuntu users happy. Favourite fix of 3.9 for sure ;)

bagong · 2017-03-06T07:28:44Z

3.9 can come 😄

mossheim · 2017-03-06T14:44:04Z

Waiting for @snappizz to add a comment about the #if as requested, and then I'll be happy to merge.

nhthn · 2017-03-06T17:21:44Z

@brianlheim done, thanks for the reminder

mossheim · 2017-03-06T17:22:11Z

Woah. Very thorough, thanks @snappizz 👍

timblechmann · 2017-03-06T18:34:44Z

editors/sc-ide/widgets/code_editor/autocompleter.cpp

+#if defined(Q_OS_UNIX) && !defined(Q_OS_MAC) && !defined(__NetBSD__)
+        QPalette p;
+        p.setColor( QPalette::Window, QColor(255, 255, 220) );
+        p.setColor( QPalette::WindowText, Qt::black );


this will break dark color schemes

Break them or have an unfavorable appearance in them? We are not able to implement configurability and theme support tooltips on linux. If you could provide it, that would be highly welcome! The current state is unacceptable for a lot of Linux users.

This is how it looks in Ubuntu Studio (xfce) with your dark scheme:

If someone would step up and make the tooltip colors more attractive, I'd much appreciate it. I have no expertise in Qt and I was just fumbling around here to fix an issue that's been bugging users for almost a year and a half.

read the colors for the color roles and fix them up. hardcoding colors should be avoided. futhermore, if it is related to gnome/gtk, netbsd would suffer the same issue

I agree that hardcoding colors is a bad idea, but I'm afraid to say I just can't dedicate the time to finding a better solution.

We would need to compare the QColor::lightness() of both QPalette::ToolTipBase and QPalette::ToolTipText, and if they are unacceptably close, QColor::darker() the lighest one and QColor::lighter the darkest one by a significant factor?

smoge · 2017-03-31T02:03:48Z

@snappizz I think uncontroversial commits like this one should be merged back into the last stable branch (3.8 in this case), where users can get the version with less bugs. Maybe a policy to follow?

nhthn added the env: SCIDE label Mar 2, 2017

nhthn added this to the 3.9 milestone Mar 2, 2017

nhthn mentioned this pull request Mar 2, 2017

Autocomplete tooltip colors are unreadable on Linux #1704

Closed

nhthn added Work In Progress (WIP) - don't merge yet comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead labels Mar 3, 2017

nhthn force-pushed the topic/linux-tooltip branch from d91e7c9 to 44e7b8e Compare March 3, 2017 00:53

nhthn removed the Work In Progress (WIP) - don't merge yet label Mar 3, 2017

nhthn force-pushed the topic/linux-tooltip branch from 44e7b8e to 9e00881 Compare March 4, 2017 02:36

editor: fix tooltip color on Linux

4384ef3

.

nhthn force-pushed the topic/linux-tooltip branch from 9e00881 to 4384ef3 Compare March 4, 2017 18:12

bagong requested review from bagong and removed request for bagong March 4, 2017 19:38

bagong approved these changes Mar 4, 2017

View reviewed changes

editor: add explanatory comment to autocomplete color directives

894c1ce

mossheim merged commit 9157fd7 into supercollider:master Mar 6, 2017

timblechmann reviewed Mar 6, 2017

View reviewed changes

nhthn deleted the topic/linux-tooltip branch March 7, 2017 05:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scide: make tooltip text black on Linux #2762

scide: make tooltip text black on Linux #2762

nhthn commented Mar 2, 2017

bagong commented Mar 2, 2017 •

edited

nhthn commented Mar 3, 2017

nhthn commented Mar 3, 2017

bagong commented Mar 3, 2017 •

edited

bagong commented Mar 3, 2017

nhthn commented Mar 4, 2017

bagong commented Mar 4, 2017

nhthn commented Mar 4, 2017

bagong left a comment

patrickdupuis commented Mar 5, 2017

nhthn commented Mar 5, 2017

bagong commented Mar 5, 2017

patrickdupuis commented Mar 6, 2017

bagong commented Mar 6, 2017

mossheim commented Mar 6, 2017

nhthn commented Mar 6, 2017

mossheim commented Mar 6, 2017

timblechmann Mar 6, 2017

bagong Mar 6, 2017

bagong Mar 6, 2017

nhthn Mar 6, 2017

timblechmann Mar 7, 2017

nhthn Mar 7, 2017

patrickdupuis Mar 7, 2017

smoge commented Mar 31, 2017 •

edited

scide: make tooltip text black on Linux #2762

scide: make tooltip text black on Linux #2762

Conversation

nhthn commented Mar 2, 2017

bagong commented Mar 2, 2017 • edited

nhthn commented Mar 3, 2017

nhthn commented Mar 3, 2017

bagong commented Mar 3, 2017 • edited

bagong commented Mar 3, 2017

nhthn commented Mar 4, 2017

bagong commented Mar 4, 2017

nhthn commented Mar 4, 2017

bagong left a comment

Choose a reason for hiding this comment

patrickdupuis commented Mar 5, 2017

nhthn commented Mar 5, 2017

bagong commented Mar 5, 2017

patrickdupuis commented Mar 6, 2017

bagong commented Mar 6, 2017

mossheim commented Mar 6, 2017

nhthn commented Mar 6, 2017

mossheim commented Mar 6, 2017

timblechmann Mar 6, 2017

Choose a reason for hiding this comment

bagong Mar 6, 2017

Choose a reason for hiding this comment

bagong Mar 6, 2017

Choose a reason for hiding this comment

nhthn Mar 6, 2017

Choose a reason for hiding this comment

timblechmann Mar 7, 2017

Choose a reason for hiding this comment

nhthn Mar 7, 2017

Choose a reason for hiding this comment

patrickdupuis Mar 7, 2017

Choose a reason for hiding this comment

smoge commented Mar 31, 2017 • edited

bagong commented Mar 2, 2017 •

edited

bagong commented Mar 3, 2017 •

edited

smoge commented Mar 31, 2017 •

edited