Spinboxes #162
Spinboxes #162
Conversation
| @@ -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); | |||
There was a problem hiding this comment.
This is not ABI compatible change.
There was a problem hiding this comment.
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)
| @@ -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); } | |||
There was a problem hiding this comment.
I want to have current Adwaita-qt GTK3 based and possible do a major GTK4 overhaul in the future.
|
|
||
| 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) { |
There was a problem hiding this comment.
I want to have current Adwaita-qt GTK3 based and possible do a major GTK4 overhaul in the future.
There was a problem hiding this comment.
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.
| @@ -29,6 +29,8 @@ | |||
|
|
|||
| #if ADWAITA_HAVE_X11 | |||
| #include <X11/Xlib-xcb.h> | |||
| // POP_OS | |||
There was a problem hiding this comment.
Yes irrelevant, it was just temporary otherwise compilation fails on POP.
src/style/adwaitastyle.cpp
Outdated
|
|
||
| // 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)); |
There was a problem hiding this comment.
Then it means it needs to be fixed above if the original height is too much.
There was a problem hiding this comment.
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 ?
src/style/adwaitastyle.cpp
Outdated
| QColor base(Colors::separatorColor(styleOptions)); | ||
| // QColor base(Colors::separatorColor(styleOptions)); | ||
| // Fix black slider ticks for dark variant | ||
| QColor base(Colors::sliderTicksColor(styleOptions)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
One final look int frameWidth(pixelMetric(PM_SpinBoxFrameWidth, option, widget)); you are expanding your frame too much in height as a result, with changing the above to int frameWidth(pixelMetric(PM_SpinBoxFrameWidth, option, widget)); size.setHeight(qMax(size.height(), int(Metrics::SpinBox_MinHeight))); as before, and still get the improved result. having a careful look your other widgets are too tall also versus gtk, but negligible. I'll check in my final changes tomorrow and leave it at that. I also did the slider ticks as per suggestion. Sorry not doing a new issue for it. Time is up. Cheers . |
Okay, slider height slightly overdone, but much improved, tweak as you like.