Skip to content

Spinboxes #162

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Spinboxes #162

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f32053a
Reduce spinbox content size. Fix Slider ticks colour.
AlexB67 Jan 3, 2022
4398b3e
undo xcb header for POP
AlexB67 Jan 3, 2022
1c23bbf
Make 3D Bevel optional. Make button style more like adwaita gtk4.
AlexB67 Jan 4, 2022
ea7ae78
Update spinbox and slider changes
AlexB67 Jan 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/lib/adwaitacolors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,14 @@ QColor Colors::separatorColor(const StyleOptions &options)
return buttonOutlineColor(options);
}

QColor Colors::sliderTicksColor(const StyleOptions &options)
{
if (ColorsPrivate::isDarkMode())
return transparentize(options.palette().color(QPalette::Text), 0.5);

return transparentize(options.palette().color(QPalette::Text), 0.2);
}

QColor Colors::toolButtonColor(const StyleOptions &options)
{
if (options.sunken() || (options.animationMode() != AnimationNone && options.animationMode() != AnimationHover)) {
Expand Down
5 changes: 5 additions & 0 deletions src/lib/adwaitacolors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ class ADWAITAQT_EXPORT Colors
* color variant
*/
static QColor separatorColor(const StyleOptions &options);
/* Relevant options:
* palette
* color variant
*/
static QColor sliderTicksColor(const StyleOptions &options);
/* Relevant options:
* palette
* mouse over
Expand Down
6 changes: 3 additions & 3 deletions src/lib/adwaitarenderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ void Renderer::renderMenuFrame(const StyleOptions &options, bool roundCorners)
options.painter()->restore();
}

void Renderer::renderButtonFrame(const StyleOptions &options)
void Renderer::renderButtonFrame(const StyleOptions &options, bool renderBevel)
{
if (!options.painter()) {
return;
Expand Down Expand Up @@ -430,8 +430,8 @@ void Renderer::renderButtonFrame(const StyleOptions &options)

// render
options.painter()->drawRoundedRect(frameRect, radius, radius);

if (!options.sunken() && options.active() && options.color().isValid()) {
// gtk4 adwaita looks flat these days, so we wont draw the extra decor by default
if (!options.sunken() && options.active() && options.color().isValid() && renderBevel) {
Copy link
Collaborator

@grulja grulja Jan 6, 2022

Choose a reason for hiding this comment

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

I want to have current Adwaita-qt GTK3 based and possible do a major GTK4 overhaul in the future.

Copy link
Author

@AlexB67 AlexB67 Jan 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Do a new new style altogether for gtk4 and leave the gtk3 one as is ? .Anyway, libadwaita has just been released, and things are a bit different again. Perhaps it should be inspired by that.

options.painter()->setPen(options.color().lighter(140));
options.painter()->drawLine(frameRect.topLeft() + QPoint(3, 1), frameRect.topRight() + QPoint(-3, 1));
options.painter()->setPen(options.outlineColor().darker(114));
Expand Down
2 changes: 1 addition & 1 deletion src/lib/adwaitarenderer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ADWAITAQT_EXPORT Renderer

static void renderMenuFrame(const StyleOptions &options, bool roundCorners = true);

static void renderButtonFrame(const StyleOptions &options);
static void renderButtonFrame(const StyleOptions &options, bool renderBevel = false);
Copy link
Collaborator

@grulja grulja Jan 6, 2022

Choose a reason for hiding this comment

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

This is not ABI compatible change.

Copy link
Author

@AlexB67 AlexB67 Jan 7, 2022

Choose a reason for hiding this comment

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

i wasn't suggesting you should incorporate it. It was a suggestion for a an improved look with a minimal change. so you can see the result. (much better IMO)


static void renderCheckBoxFrame(const StyleOptions &options);

Expand Down
2 changes: 1 addition & 1 deletion src/lib/stylesheet/processed/Adwaita-dark.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
@define-color focus_border_color rgba(21, 83, 158, 0.7);
@define-color alt_focus_border_color rgba(255, 255, 255, 0.3);
@define-color dim_label_opacity 0.55;
button { color: #eeeeec; outline-color: rgba(21, 83, 158, 0.7); border-color: #1b1b1b; background-image: linear-gradient(to top, #373737 2px, #3a3a3a); box-shadow: 0 1px 2px rgba(0, 0, 0, 0.07); }
button { color: #eeeeec; outline-color: rgba(21, 83, 158, 0.7); border-color: #1b1b1b; background-image: image(#393939); box-shadow: 0 1px 2px rgba(0, 0, 0, 0.07); }
Copy link
Collaborator

@grulja grulja Jan 6, 2022

Choose a reason for hiding this comment

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

I want to have current Adwaita-qt GTK3 based and possible do a major GTK4 overhaul in the future.


button:hover { color: #eeeeec; border-color: #1b1b1b; background-image: linear-gradient(to top, #303030 20%, #323232 90%); box-shadow: 0 1px 2px rgba(0, 0, 0, 0.07); }

Expand Down
2 changes: 2 additions & 0 deletions src/style/adwaitahelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

#if ADWAITA_HAVE_X11
#include <X11/Xlib-xcb.h>
// POP_OS
Copy link
Collaborator

@grulja grulja Jan 6, 2022

Choose a reason for hiding this comment

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

Unrelated change.

Copy link
Author

@AlexB67 AlexB67 Jan 6, 2022

Choose a reason for hiding this comment

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

Yes irrelevant, it was just temporary otherwise compilation fails on POP.

// #include <xcb/xcb.h>
#endif

namespace Adwaita
Expand Down
10 changes: 7 additions & 3 deletions src/style/adwaitastyle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2778,10 +2778,12 @@ QSize Style::spinBoxSizeFromContents(const QStyleOption *option, const QSize &co
size.rwidth() += Metrics::Button_ItemSpacing;

// FIXME this shouldn't be needed but apparently some width is still missing
size.rwidth() += size.height() / 2;
// reduce to 1 / 4 sems enough, was 1 / 2
size.rwidth() += size.height() / 4;

// set minimum size
size.setHeight(qMax(size.height(), int(Metrics::SpinBox_MinHeight)));
// size.height() appears way too much - so removed
size.setHeight(int(Metrics::SpinBox_MinHeight));
Copy link
Collaborator

@grulja grulja Jan 6, 2022

Choose a reason for hiding this comment

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

Then it means it needs to be fixed above if the original height is too much.

Copy link
Author

@AlexB67 AlexB67 Jan 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

No comprendo :) edit: I think I know, you mean what size.height() returns. If so yes, I had a play, but didn't quite get far enough to investigate why that wouldn't behave as I wanted. The same logic works okay elsewhere for other widgets, clearly for the spinbox it does something wrong. values in adwaita.h for minheight seem normal as other widgets,

SpinBox_FrameWidth = LineEdit_FrameWidth, SpinBox_ArrowButtonWidth = 20, SpinBox_MinHeight = 36, SpinBox_MinWidth = 80, SpinBox_MarginHeight = 4, SpinBox_MarginWidth = 8,

so not sure. you tell me :) Is that extra logic really needed in the qmax statement , even though it appears everywhere. note there is a also a comment in the code saying the spacing is wrong and extra width needed to be added. Why was that needed when it shouldn't have, perhaps there is a deeper issue here ?

size.setWidth(qMax(size.width(), int(Metrics::SpinBox_MinWidth)));

return size;
Expand Down Expand Up @@ -6751,7 +6753,9 @@ bool Style::drawSliderComplexControl(const QStyleOptionComplex *option, QPainter
StyleOptions styleOptions(palette, _variant);

// colors
QColor base(Colors::separatorColor(styleOptions));
// QColor base(Colors::separatorColor(styleOptions));
// Fix black slider ticks for dark variant
QColor base(Colors::sliderTicksColor(styleOptions));
Copy link
Collaborator

@grulja grulja Jan 6, 2022

Choose a reason for hiding this comment

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

Maybe just use:
QColor base(Colors::transparentize(palette.text(), 0.5))

Without introducing a new public method. Also, this is an unrelated change to spinboxes so maybe put it into a separate review.

Copy link
Author

@AlexB67 AlexB67 Jan 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, agreed, good idea· but I don't think I'll have the time in the foreseeable future. I had a play over xmas, it's a far as I got, now back to work

For me it doesn't matter because it a common bug in a lot of qt dark themes with slider ticks (kvantum gets it right though) So I implemented my own derived slider widget, override the paint event so it works with all themes now, so no more problems.

As for the changes I run it on POP with the above changes, to me it already looks much better, so. I am happy, and took the time to report it ... BUT ...
Since you know the code base (I don't). I personally do think the slider and spinboxes should be fixed in whatever way you see fit.

I leave it to you. I raised the bug, no doubt for you it will be trivial to fix it in whatever way you see fit.

Or

you can wait, whenever I have the incentive and/or time to dig in, could be .. whenever. Doing themes is not exactly my favourite hobby, but i hate it when they don't work as they should.


while (current <= sliderOption->maximum) {
// adjust color
Expand Down