Removing hardcoded stylesheet changes

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Post Reply
User avatar
chennes
Posts: 1336
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Removing hardcoded stylesheet changes

Post by chennes »

In working with stylesheet authors over the last couple of months, I've run into some calls to setStylesheet() in the source code, often with hardcoded colors and/or margins. Here's the current state (excluding the QSint stuff, which is a whole 'nother deal):

Code: Select all

Gui/DlgMacroExecuteImp.cpp: groupBox7->setStyleSheet(QString::fromLatin1("QGroupBox::title {color:red}"));
Gui/DlgMacroExecuteImp.cpp: buttonAddAction->setStyleSheet(QString::fromLatin1("color:red"));
Gui/DlgMacroExecuteImp.cpp: moveActionRightButton->setStyleSheet(QString::fromLatin1("background-color: red"));
Gui/DlgMacroExecuteImp.cpp: newButton->setStyleSheet(QString::fromLatin1("color:red"));
Gui/DlgMacroExecuteImp.cpp: label->setStyleSheet(QString::fromLatin1("color:red"));
Gui/DlgParameterImp.cpp: ui->findGroupLE->setStyleSheet(QString());
Gui/DlgParameterImp.cpp: ui->findGroupLE->setStyleSheet(styleSheet);
Gui/InputField.cpp: iconLabel->setStyleSheet(QString::fromLatin1("QLabel { border: none; padding: 0px; }"));
Gui/InputField.cpp: setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px } ").arg(iconLabel->sizeHint().width() + frameWidth + 1));
Gui/MainWindow.cpp: statusBar()->setStyleSheet(QString::fromLatin1("#statusBar{}"));
Gui/MainWindow.cpp: statusBar()->setStyleSheet(d->status->err);
Gui/MainWindow.cpp: statusBar()->setStyleSheet(d->status->wrn);
Gui/MainWindow.cpp: statusBar()->setStyleSheet(QString::fromLatin1("#statusBar{}"));
Gui/MainWindow.cpp: statusBar()->setStyleSheet(d->status->msg);
Gui/SelectionView.cpp: clearButton->setStyleSheet(QString::fromUtf8("QToolButton {margin-bottom:1px}"));
Gui/SpinBox.cpp: lineedit->setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px } ").arg(iconLabel->sizeHint().width() + frameWidth + 1));
Gui/Widgets.cpp: clearButton->setStyleSheet(QString::fromLatin1("QToolButton { border: none; padding: 0px; }"));
Gui/Widgets.cpp: setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px; } ")
Gui/Widgets.cpp: setStyleSheet(QString::fromLatin1("Gui--UrlLabel {color: #0000FF;text-decoration: underline;}"));
Gui/Widgets.cpp: setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px } ").arg(iconLabel->sizeHint().width() + frameWidth + 1));
Mod/Arch/ArchAxis.py: self.table.setStyleSheet(style)
Mod/Draft/DraftGui.py: self.baseWidget.setStyleSheet(style)
Mod/Draft/DraftGui.py: self.baseWidget.setStyleSheet(
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString::fromLatin1("QLabel{color: red;}"));
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString());
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString::fromLatin1("QLabel{color: green;}"));
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString::fromLatin1("QLabel{color: red;}"));
Mod/Path/PathScripts/PathGui.py: widget.setStyleSheet("color: black")
Mod/Path/PathScripts/PathGui.py: widget.setStyleSheet("color: gray")
A few of these calls are protected by an if (qApp->styleSheet().isEmpty()) conditional, but most are not. The problem with hardcoded colors is pretty obvious once you get user stylesheets involved. I think the best path for fixing those is to have a set of preferences for those colors, and basically leave the code as-is except swapping in the user-set color choice. This will allow Preference Pack authors to distribute those colors alongside a Qt CSS stylesheet, and won't break any of the code logic that's changing the colors based on some event. Using the conditional there is ineffective because we're using color to represent some state, and to my knowledge that's not something that's available via the globally-loaded stylesheet.

For the padding, it's probably best to use the conditional to guard it. The problem with having it hardcoded is that it's frustrating to stylesheet authors who need to change it, and their changes either have no effect, or have counterintuitive results (e.g. setting the width to some number, and it ends up one higher than that, etc.). This will probably require changes to existing stylesheets, but in many cases those sheets aren't really being maintained and are a bit broken anyway.

Does my suggestion of adding preferences for those colors, and conditionals for the paddings, make sense? Does anyone foresee other problems with that plan?
Chris Hennes
Pioneer Library System
adrianinsaval
Posts: 805
Joined: Thu Apr 05, 2018 5:15 pm

Re: Removing hardcoded stylesheet changes

Post by adrianinsaval »

I can't look closely at any of it right now so I don't know if this gives any issue at all, but I would ask that those that work on this take into account what happens when someone has not set any stylesheet in FreeCAD but has a system wide theme (KDE users like me most likely)
wmayer
Site Admin
Posts: 17748
Joined: Thu Feb 19, 2009 10:32 am

Re: Removing hardcoded stylesheet changes

Post by wmayer »

Gui/DlgParameterImp.cpp: ui->findGroupLE->setStyleSheet(QString());
Gui/DlgParameterImp.cpp: ui->findGroupLE->setStyleSheet(styleSheet);
This one is required here. In the parameter editor there is a search box at the bottom where you can do a quick search for parameter groups. If there is no matching then the background of the line edit becomes light-red to indicate it visually. Changing the background color with QPalette only works if no application-wide style sheet is set but has no effect as soon one is set. So, the only way is to set/override the style sheet of a single widget. This shouldn't cause any issues with the QSS files.
Gui/InputField.cpp: iconLabel->setStyleSheet(QString::fromLatin1("QLabel { border: none; padding: 0px; }"));
Gui/InputField.cpp: setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px } ").arg(iconLabel->sizeHint().width() + frameWidth + 1));
Gui/SpinBox.cpp: lineedit->setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px } ").arg(iconLabel->sizeHint().width() + frameWidth + 1));
Gui/Widgets.cpp: clearButton->setStyleSheet(QString::fromLatin1("QToolButton { border: none; padding: 0px; }"));
Gui/Widgets.cpp: setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px; } ")
Gui/Widgets.cpp: setStyleSheet(QString::fromLatin1("QLineEdit { padding-right: %1px } ").arg(iconLabel->sizeHint().width() + frameWidth + 1));
They are needed too. Without them the input field widget could get squashed by the layout of a parent widget so that the text could be truncated.
Gui/MainWindow.cpp: statusBar()->setStyleSheet(QString::fromLatin1("#statusBar{}"));
Gui/MainWindow.cpp: statusBar()->setStyleSheet(d->status->err);
Gui/MainWindow.cpp: statusBar()->setStyleSheet(d->status->wrn);
Gui/MainWindow.cpp: statusBar()->setStyleSheet(QString::fromLatin1("#statusBar{}"));
Gui/MainWindow.cpp: statusBar()->setStyleSheet(d->status->msg);
This affects the colors of the status bar. However, the colors are hard-coded and may conflict an application-wide style sheet. In case of problems this should be improved.
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString::fromLatin1("QLabel{color: red;}"));
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString());
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString::fromLatin1("QLabel{color: green;}"));
Mod/Part/Gui/TaskAttacher.cpp: ui->message->setStyleSheet(QString::fromLatin1("QLabel{color: red;}"));
It's used by the attachment dialog. The colors probably won't conflict with an application-wide style sheet.
Gui/Widgets.cpp: setStyleSheet(QString::fromLatin1("Gui--UrlLabel {color: #0000FF;text-decoration: underline;}"));
This should be OK. The URL label only shows a blue underlined text to indicate a link if no style sheet is set.
User avatar
onekk
Posts: 1285
Joined: Sat Jan 17, 2015 7:48 am
Contact:

Re: Removing hardcoded stylesheet changes

Post by onekk »

Same thing will happen for other default quantities and settings.

I have faced some problem as example in QTFile dialog, when I've changed "file ordering" in an external app, and found that even in FreeCAD that is running thorough AppImage (I thought isolated from the system) have taken this settings for his file open dialog.

Same for as example Navigation Cube, on which I can't obtain a decent character for labels.

In my opinion there will be some ways to determine things, maybe using some setting in ./FreeCAD/user.cfg or other places to make more "predictable" results as not every people use KDE in Linux (I mostly use GTK apps) and maybe have a checkbox to permit the use of a "custom" definition for FreeCAD only.

This way even in an AppImage you could have a "better isolation" from the underlying OS that I think is one of the goal in creating an AppImage.

Thanks for your the invaluable work.

Regards

Carlo D.
Il mio Blog: https://onekk-maker.blogspot.com/

Introduzione a FreeCAD in Italiano: https://github.com/onekk/freecad-doc
adrianinsaval
Posts: 805
Joined: Thu Apr 05, 2018 5:15 pm

Re: Removing hardcoded stylesheet changes

Post by adrianinsaval »

the purpose of the appimage is to get the dependencies bundled with the application, there's no isolation of settings and other data
User avatar
onekk
Posts: 1285
Joined: Sat Jan 17, 2015 7:48 am
Contact:

Re: Removing hardcoded stylesheet changes

Post by onekk »

yes and no, you could obtain a decent isolation setting some system variables with a launch script, so you could have many FreeCAD istances running and using a different settings directory for each one.

Having some system settings saved in the. an arbitrary /.FreeCAD directory will be a very useful solution even for developers or skilled users.

Said so Navigation Cube character should be modifiable by the user (I have managed to get it working only with LS3 version where there is a proper font setting in preferences), addons will gave error and my GitHub request signaling the error is unaswered.

Regards

Carlo D.
Il mio Blog: https://onekk-maker.blogspot.com/

Introduzione a FreeCAD in Italiano: https://github.com/onekk/freecad-doc
adrianinsaval
Posts: 805
Joined: Thu Apr 05, 2018 5:15 pm

Re: Removing hardcoded stylesheet changes

Post by adrianinsaval »

You can, but you can also do that with a regular packaged application, I'm just saying that this is not the purpose Appimages where made for (maybe you are confusing with snaps?) so you shouldn't expect isolated configs for stuff you haven't isolated yourself like the file dialog
Post Reply