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

desktop: remove special QGroupBox stylesheet for non-Windows systems #2748

Merged
merged 1 commit into from Apr 13, 2020

Conversation

bstoeger
Copy link
Collaborator

That style-sheet made things look really ugly on most Linux themes.

Signed-off-by: Berthold Stoeger bstoeger@mail.tuwien.ac.at

Describe the pull request:

  • Bug fix
  • Functional change
  • New feature
  • Code cleanup
  • Build system change
  • Documentation change
  • Language translation

Pull request long description:

Fixes an uglification of the the DiveInfoTab on Linux.

Changes made:

  1. Remove special non-Windows QGroupBox stylesheet.

Mentions:

@willemferguson: It seems you wrote that stylesheet for Windows, but actually it was only included for non-Windows?

That style-sheet made things look really ugly on most Linux themes.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Copy link
Collaborator

@dirkhh dirkhh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly we want either this or an inversion of the condition.
Give me a moment to try that.

@@ -38,11 +38,7 @@ TabDiveInformation::TabDiveInformation(QWidget *parent) : TabBase(parent), ui(ne
connect(ui->diveType, SIGNAL(currentIndexChanged(int)), this, SLOT(diveModeChanged(int)));
QString CSSSetSmallLabel = "QLabel { color: mediumblue; font-size: " + // Using label height
QString::number((int)(0.5 + ui->diveHeadingLabel->geometry().height() * 0.66)) + "px;}"; // .. set CSS font size of star widget subscripts
#if defined(Q_OS_WIN)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if the suspicion was that this check is inverted (i.e., that the other style sheet is needed on Windows), have you given that a try (err, do you have access to a Windows system to try it)?
I need to start up my Windows VM, deal with an hour of mandatory updates and then see if one loops better than the other before merging this (yes, I realize that this wasn't used on Windows in the current code - I'm wondering if this would be a net improvement if it WAS used)

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 13, 2020

Ugh. Wow. Apparently I tested the German localization in that VM last time I ran Subsurface there :-)
Anyway, the look without the customization is kinda wasteful and really bad. So that's what we get with this PR (and what's the standard already today). Clearly room for improvement.
The second screenshot is with PR #2754 -- yeah, let's go with yours :-)

(also, the German translations for some of these terms is REALLY verbose and long...)

Screen Shot 2020-04-13 at 10 09 55 AM

Screen Shot 2020-04-13 at 10 10 21 AM

@dirkhh dirkhh merged commit b4e0968 into subsurface:master Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants