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.

Sign up for GitHub

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

Jump to bottom

Add non-native dark theme #5076

Closed
wants to merge 1 commit into from
Closed

Add non-native dark theme #5076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/app/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ struct QBtCommandLineParameters
#endif
#ifndef DISABLE_GUI
bool noSplash;
bool darkTheme;
#else
bool shouldDaemonize;
#endif
Expand All @@ -112,6 +113,7 @@ struct QBtCommandLineParameters
#endif
#ifndef DISABLE_GUI
, noSplash(Preferences::instance()->isSplashScreenDisabled())
, darkTheme(false)
#else
, shouldDaemonize(false)
#endif
Expand Down Expand Up @@ -187,6 +189,19 @@ int main(int argc, char *argv[])
std::cerr << "Couldn't set environment variable...\n";

#ifndef DISABLE_GUI

// Set dark theme
if (params.darkTheme) {
QFile file(":/qdarkstylesheet.qss");
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be more useful to put the stylesheet outside of the application binary to allow for possible user edits?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, It would be more useful, but we must guarantee that stylesheet will be deployed with application separately. Should it be in %Some_user_settings_path% or in app directory?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry but I'm not familiar with Windows filesystem layout. Certainly application directory is a bad place, since in general user shall not be able to edit files there. It might have sense to keep the file in resources and copy it (without overwriting) into the user directory at start up.

Although the maintainer in one of the PRs told me that he objects colour customisation. Be ready ;)

Copy link
Author

Choose a reason for hiding this comment

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

Does "to objecting" mean "to protesting"? So, when I finish stylesheet integration, may it won't be approved like a customization patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know :) I'm into customisations and support the idea that applications styles have to be controllable by a user. @sledgehammer999 ? Could you comment on this, please?

if(file.open(QIODevice::ReadOnly | QIODevice::Text)) {
Copy link
Member

Choose a reason for hiding this comment

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

Fix coding style!

Copy link
Author

Choose a reason for hiding this comment

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

Ok) I'll look to coding style guide closely and properly and fix these mistakes. Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a config for uncrustify which can help you at least partially. The config is not complete yet, and you are welcomed to extend it, of course.

app->setStyleSheet(file.readAll());
file.close();
}
QPalette palette = app->palette();
palette.setColor(QPalette::Active, QPalette::Base, QColor(100,100,100));
app->setPalette(palette);
}

if (!userAgreesWithLegalNotice())
return EXIT_SUCCESS;
#else
Expand Down Expand Up @@ -297,6 +312,9 @@ QBtCommandLineParameters parseCommandLine()
else if (arg == QLatin1String("--no-splash")) {
result.noSplash = true;
}
else if (arg == QLatin1String("--dark-theme")) {
result.darkTheme = true;
}
#else
else if ((arg == QLatin1String("-d")) || (arg == QLatin1String("--daemon"))) {
result.shouldDaemonize = true;
Expand Down
2 changes: 2 additions & 0 deletions src/gui/mainwindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ MainWindow::MainWindow(QWidget *parent)
createKeyboardShortcuts();
// Create status bar
status_bar = new StatusBar(QMainWindow::statusBar());
this->setStyleSheet("QStatusBar::item { border-width: 0; }");
Copy link
Member

Choose a reason for hiding this comment

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

This is C++! this is implicit. No need to use it everywhere.


connect(status_bar->connectionStatusButton(), SIGNAL(clicked()), SLOT(showConnectionSettings()));
connect(actionUse_alternative_speed_limits, SIGNAL(triggered()), status_bar, SLOT(toggleAlternativeSpeeds()));

Expand Down
2 changes: 0 additions & 2 deletions src/gui/statusbar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@
StatusBar::StatusBar(QStatusBar *bar)
: m_bar(bar)
{
qApp->setStyleSheet("QStatusBar::item { border-width: 0; }");

Preferences* const pref = Preferences::instance();
Copy link
Member

Choose a reason for hiding this comment

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

I just did the move in e35a7ef. Placing here is more intuitive IMO, or, this is blocking this PR?

Copy link
Author

Choose a reason for hiding this comment

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

I agree with you about intuitive, but it is blocking dark style sheets - I tested it several times with and without it)

connect(BitTorrent::Session::instance(), SIGNAL(speedLimitModeChanged(bool)), this, SLOT(updateAltSpeedsBtn(bool)));
m_container = new QWidget(bar);
Expand Down