D28651: Load and use global animation settings

hpereiradacosta added a comment. Thanks for this change ! Can the same be done in the breeze window decoration ? It is strange to have an animation settings there and not in the style. Hugo REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D28651 To: sandsmark, #b

D28651: Load and use global animation settings

hpereiradacosta added a comment. I have no idea how this is supposed to work. But the current fix overwrites what's in the breeze configutation, right ? Would it not lead to utter confusion ? These settings should be set at one place and one only. Whether breeze or global I have no strong

D27669: [kstyle] Tools area

hpereiradacosta added a comment. Hi I don't think it is very satisfactory to say that the perforrmance hit it is "the apps fault" ... It is not there without the change, and with other widgets styles (or is it ? Did any one check, e.g. oxygen which is quite resource heavy because of gradi

D27669: [kstyle] Tools area

hpereiradacosta added a comment. In D27669#637164 , @ndavis wrote: > I've discovered another bug. When you move a window by dragging on an empty area, all hover effects stop working. This bug has been here since forever and is not related

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breezehelper.cpp:1687 > +while (parent != nullptr) { > +if (qobject_cast(widget) || qobject_cast QDockWidget*>(widget)) { > +return false; mmm shouldn't you test on parent rather than widget here ? REPO

D27669: [kstyle] Tools area

hpereiradacosta added a comment. In D27669#634758 , @ngraham wrote: > So I'm still having an issue with the feature um, not working. :( I have it turned on in the Breeze settings but it behaves as if off; that is to say, I see no different appea

D27669: [kstyle] Tools area

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. All good, as far as I am concerned. Thanks ! REPOSITORY R31 Breeze BRANCH cblack/toolsarea REVISION DETAIL https://phabricator.kde.org/D27669 To: cblack, #plasma,

D27669: [kstyle] Tools area

hpereiradacosta added a comment. I finally got some time to look at the code. Some minor comments below (compiler warnings) Also there is a problem with menubar text color when option is disabled. Here at least it is still set to the decoration color, leading to invisible text with default

D27669: [kstyle] Tools area

hpereiradacosta added a comment. Hi Carson, Thanks for the update. At first glance this all look pretty good. I should be able to do some more in depth testing and code review by the end of tomorrow. Hugo REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D27669 To:

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > cblack wrote in breeze.h:104 > The tools area's separator has to separate two large areas of the window and > thus should be stronger than the separators that only have to separate > borderless and backgroundless buttons. As I said, to m

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > cblack wrote in breeze.h:104 > With: F8185479: image.png > Without: F8185480: image.png Sorry for the many postings, I had another unrelated comment on these sc

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > cblack wrote in breezetoolsareamanager.cpp:133 > This only triggers when the window has no items in the tools area—it's used > in conjunction with line 878 of breezestyle.cpp in order to render a border > at the top of the window. Clear

D27669: [kstyle] Tools area

hpereiradacosta added a comment. In D27669#630974 , @ngraham wrote: > Nice, the colors are better now. I still see a difference in animation speed when the titlebar and toolbar change color though. It's especially visible with the current Breeze

D27669: [kstyle] Tools area

hpereiradacosta added a comment. Hi Carlston, Thanks for the updated patch and screenshot. First I agree that the new (latest) checks on whether the toolbar palette was changed or not are much more elegant and just as efficient as the previous implementation. Second, some more comment

D27669: [kstyle] Tools area

hpereiradacosta added a comment. Hi, finally had some time to double-check into kiconloader, and confirmed that setting the custom palette and reseting is pretty harmless since it does not invalidate the cache. So to me it is fine to leave this part as it is now. I have added a few more

D28087: Fix Defaults not being set properly in Breeze window decoration settings for 'Draw a circle around close button'

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. Many thanks ! REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D28087 To: paulm, #breeze, hpereiradacosta Cc: plasma-devel, Orage, LeGast00n, The-Fer

D27669: [kstyle] Tools area

hpereiradacosta added a comment. Generating the toolarea colors from the windows colors and making sure the WindowText color still works; is also a solution. But it requires to also change the decoration to use this generated color instead of that of a palette, and an option to go with it. A

D27669: [kstyle] Tools area

hpereiradacosta added a comment. For what its worth: what you would need to fix this properly, is a new QIcon::Mode for "in active tool area" that you would map the the right colors when generating a given pixmap from the svg inside kiconloader. Only then would you be able to cache for a g

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > hpereiradacosta wrote in breezestyle.cpp:4382 > I'll dig in KIconLoader code a bit over the week-end to see what really > happens when calling setCustomPalette and resetPalette, to get a sense of how > resource consuming this is ... So,

D27892: [RFC] Don't draw shadows on quick tiled or maximized edges

hpereiradacosta added a comment. In D27892#623436 , @davidre wrote: > In D27892#623425 , @ngraham wrote: > > > +1 for the concept and resulting appearance. But, should this maybe be done in KWin ins

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > hpereiradacosta wrote in breezestyle.cpp:4382 > I am sorry to say this is a serious show stopper. > One cannot reset/update the kiconloader palette at every repaint event for > every single toolbutton in toolbars especially when there are

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > cblack wrote in breezestyle.cpp:4382 > The issue is that the QStyle only has the global icon palette to mutate for > the tools area. If the custom palette were to be left intact, that would > affect widgets it's not supposed to. There is

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breezestyle.cpp:4897 > + > +auto palette = toolbar->palette(); > +palette.setColor( QPalette::Window, > _toolsAreaManager->background(widget) ); This should move in the if condition below. Creating a new palette (even by

D27669: [kstyle] Tools area

hpereiradacosta added a comment. I noticed there seems to be something wrong in the color logic when - active window decoration color is the same as the main window decoration - inactive window decoration is different (essentially the opposite as current breeze theme) In that case

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > cblack wrote in breeze.h:104 > This is part of the visual changes of the tools area design—extra spacing as > to prevent ugly border on border action. Why would this be more "ugly" as when there was no toolarea ? can you post a screeshot

D27669: [kstyle] Tools area

hpereiradacosta added a comment. A few more comments, but all in all seems to be getting there (beside the options to disable and/or to define the colors based on the QPalette and not the decorations) INLINE COMMENTS > breezehelper.cpp:1631 > + > +QMainWindow* window; > +

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breezehelper.cpp:1613 > + > +auto castedWidget = const_cast(widget); > + as far as I can tell you do not need the const_cast. just check the relevant methods to take a const as input. Const_cast must really be avoided as much as

D27669: [kstyle] Tools area

hpereiradacosta added a comment. In D27669#625338 , @ngraham wrote: > FWIW, I'm okay making this configurable, as I do admit that it could be a very polarizing change. We do intend to change the default Breeze color scheme itself to use a light

D27669: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breezestyle.cpp:4382 > +QPixmap pixmap = toolButtonOption->icon.pixmap( iconSize, > iconMode, iconState ); > +if (_helper->isInToolsArea(widget)) { > +KIconLoader::global()->setCustomPalette(widget->

D27669: [kstyle] Tools area

hpereiradacosta added a comment. Some more comments about the code. INLINE COMMENTS > breeze.h:104 > // toolbars > -ToolBar_FrameWidth = 2, > +ToolBar_FrameWidth = 6, > ToolBar_HandleExtent = 10, This change seems unrelated to introducing a tool area. I wou

D27669: [kstyle] Tools area

hpereiradacosta added a comment. In D27669#625270 , @IlyaBizyaev wrote: > For color schemes that have contrasting titlebar and content colors, this patch makes application headers look giant... > F8168755: photo_2020-03-10_15-09-54.jpg

D27669: [kstyle] Tools area

hpereiradacosta added a comment. Some more comments, mostly code related. Thanks for addressing all the comments so far ! INLINE COMMENTS > breezehelper.cpp:33 > +#include > +#include > This include is not needed any more as far as I can tell > breezehelper.cpp:1723 > + > +p

D27669: [kstyle] Tools area

hpereiradacosta added a comment. The toolbar background seems to be broken when there are multiple toolbars in the same row. See, e.g. ktorrent: F8168244: Screenshot_20200309_213717.png Does not happen for applications that have a single toolbar (m

D27938: 'Classic' and 'Redmond' button icon styles, configurable via Breeze window decoration settings

hpereiradacosta added a comment. In my opinion this should really go into a different decoration. Starting to have "styles inside styles" inside a single decoration is a UI mess. If you want to turn breeze into a theme engine and not a theme, then so be it, but one must go the full way. No

D27669: WIP: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > hpereiradacosta wrote in breezehelper.cpp:1726 > Then shouldn't you divide device pixel ratio ? > Also, someone else should double check. To me that does not make sense. > Everything else (*everything*) scales with device pixel ratio. Why

D27669: WIP: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breezetoolsareamanager.cpp:146 > +if (window == widget->window()->windowHandle()) { > +hasWidget = true; > +} you can break here once one widget is found. no need to loop over the remaini

D27669: WIP: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breeze.h:191 > +AnimationPressed = 0x8, > +AnimationWindowFocused = 0x10, > }; Sorry for the many postings. As far as I can tell this guy is used nowhere. Please remove. REPOSITORY R31 Breeze REVISION DETAIL htt

D27669: WIP: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breezetoolsareamanager.h:1 > +#pragma once > + Please use garding defines rather than "pragma once" for consistency with the rest of the code. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D27669 To: cblack, #pl

D27669: WIP: [kstyle] Tools area

hpereiradacosta added a subscriber: davidedmundson. hpereiradacosta added a comment. also adding @davidedmundson in the loop in particular for opinion on the devicepixel ratio business, and in case he can reproduce the crash. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.

D27669: WIP: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > cblack wrote in breezehelper.cpp:1726 > In testing, I found that this would remain 1 physical pixel regardless of > scale factor. Then shouldn't you divide device pixel ratio ? Also, someone else should double check. To me that does not

D27669: WIP: [kstyle] Tools area

hpereiradacosta added inline comments. INLINE COMMENTS > breezehelper.cpp:1726 > + > +painter->setPen(QPen(outline, > 1*widget->screen()->devicePixelRatio())); > +painter->setBrush(Qt::NoBrush); No. Should be QPen( outline, 1) or just "outline" DevicePixelRatio is handled by pai

D27669: WIP: [kstyle] Tools area

hpereiradacosta added a comment. In D27669#619991 , @cblack wrote: > In D27669#619230 , @hpereiradacosta wrote: > > > right now it crashes all applications (here at least) when you unlock toolbars,

D27669: WIP: [kstyle] Tools area

hpereiradacosta added a comment. Overall, this is good stuff I think (although we should also discuss whether this should be part of breeze or rather a next-gen theme). Still: right now it crashes all applications (here at least) when you unlock toolbars, move them to a different position

D26823: Port connections to new syntax

hpereiradacosta added a comment. In D26823#599100 , @davidedmundson wrote: > > yeah ok. Just noted this. indeed. > > Sorry, I assumed you had seen the thread. > Hope you're ok with it? Sure thing ! The whole thing is more modern an

D27000: [kstyle] Drop Helper::connection()

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. thanks ! REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D27000 To: zzag, hpereiradacosta Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jral

D26978: [kstyle] Use QX11Info::isCompositingManagerRunning()

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. Makes sense, thanks ! In principle, the removal of Helper::connection, although is a change unrelated to the commit purpose. Maybe put it in a different commit ? (t

D26823: Port connections to new syntax

hpereiradacosta added a comment. In D26823#599003 , @zzag wrote: > In D26823#598999 , @hpereiradacosta wrote: > > > This breaks the KDE4 compilation of breeze style (of course !) > > > Qt 4 bui

D26823: Port connections to new syntax

hpereiradacosta added a comment. To compile with kde4: cmake -DUSE_KDE4=1 I know that some distributions (Fedora31 which I am using) do ship the kde4 version of the breeze style. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D26823 To: davidedmundson, #plasma

D26823: Port connections to new syntax

hpereiradacosta added a comment. This breaks the KDE4 compilation of breeze style (of course !) So if this patch is to stay, then one should officially drop the kde4 support (or make a branch for it). In that case, much more code can go (a lot of QT Version ifdefs, some CMake magic, etc.)

D26783: Center only during drawing, not the hit rects

hpereiradacosta added a comment. Thanks for the fix. On a different topics, I have found a couple more places where the separators look weird. See: F7895454: Screenshot_20200121_102848.png or F7895458: Screenshot_20200121_102934.png

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. > Can the top of the scrollbar go below the column headers instead of next to them? > > Like this: > F7886305: Screenshot_20200116_083629.png I don't think it is doable inside Qt no, due to how widgets are ne

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. As a side note: I have been using this code for a couple of days now, and while I think the separator looks well for vertical scrollbars, when you have both vertical and horizontal, ... this will need some getting used to. See: F7886206: Screenshot_20200

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. In D26655#595314 , @mart wrote: > In D26655#595102 , @ndavis wrote: > > > I noticed that when hovering on the scrollbar border, while the view area is not focused, th

D26655: show a thin separator between view and scrollbar

hpereiradacosta accepted this revision. hpereiradacosta added a comment. Confirmed that the centering is fixed. Now the arrow seem to have moved vertically (for vertical scrollbars) by half a pixel, which is probably a consequence of the 20->21, and make them look somewhat thicker due to ant

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. yeah thanks ! Now, it turns out that the arrows are still ill-aligned (with respect to the handle). Maybe they use the full width for rendering ? REPOSITORY R31 Breeze BRANCH arcpatch-D26655 REVISION DETAIL https://phabricator.kde.org/D26655 To: mar

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. Alignment again: so you increased the width of the groove rather than increasing the width of the scrollbar. This indeed fixes the alignment of the handle (and groove), but now the scrollbar arrows are off centered. (for the same reason), with both the scroll

D26655: show a thin separator between view and scrollbar

hpereiradacosta added inline comments. INLINE COMMENTS > breezestyle.cpp:6570 > + > +_helper->renderScrollBarBorder( painter, separatorRect, > _helper->alphaColor( option->palette.color( QPalette::WindowText ), 0.1 )); > + Another thing: Color role should be QPalette::Text rather than W

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. Hi again Marco, since you mentionned centering in plasmoids in the other commits, in fact here also the centering is wrong, with the added separator. Probably the offset is the one pixel you take for the separator. Can you double check ? (this appears true

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. >> my complain here is that this argument was either not made or discarded, when the first switch to thin scrollbar was done. >> This is my main concern about this change: the going back and forth using adhoc arguments each time to justify the change, often

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. In D26655#594280 , @mart wrote: > In D26655#594257 , @hpereiradacosta wrote: > > > Is this really intended ? > > I would at least keep the color blending, making

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. I would start by - rebase this patch to apply cleanly - split it in two (one for the separator line and one for the think handle removal), because these are really two features, adressing two different issues: 1: the floating bar 2: the two small

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. Removed previous comment because after testing the patch it turns out that this is not too strong of an issue. However I also noticed that you not only removed the think handle but also the color blending. This make the handle not only larger but also stron

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. Also, concerning the separator line: how does it look with hovered scrollbars, when the handle grove is also rendered. Isn't there some redundancy between the handle groove and the separator ? (essentially a frame inside a frame) should the handle groove

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. In D26655#594240 , @ngraham wrote: > Regarding the thickened scrollbars, I think it makes sense for a few reasons: > > - Acknowledging user feedback: we've had a bunch of complaints about the thin scrollbars.

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. In D26655#594173 , @ahiemstra wrote: > > In QQC there is an issue of overlap between item selection (and header titles) and scrollbar. Isn't that made even worth with the addition of the thin separator ? > >

D26655: show a thin separator between view and scrollbar

hpereiradacosta added a comment. Hi Notmart, you need to rebase (you have some unrelated changes in the diff, which revert latest change from Noah) - Concerning the separator, it looks great. - Concerning reverting the thin bar, I think this is really unfortunate. I expect you will h

D26639: Make checkboxes/radiobuttons use Window Background in windows and View Backround in lists

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. In D26639#593537 , @ndavis wrote: > In D26572#593511 , @hpereiradacosta wrote:

D26572: Always render checkbox/radiobutton background

hpereiradacosta added a comment. In D26572#593496 , @ndavis wrote: > What I meant is that I did not change the color of the checkbox background in this patch. I only made it so that the background would always be rendered. Clear enough. I

D26572: Always render checkbox/radiobutton background

hpereiradacosta added a comment. In D26572#593442 , @ndavis wrote: > In D26572#593379 , @hpereiradacosta wrote: > > > in more detail: imagine a color scheme where window background is black, window

D26572: Always render checkbox/radiobutton background

hpereiradacosta added a comment. in more detail: imagine a color scheme where window background is black, window text is white, view background is white view text is black. you will get a white square on a white background for unchecked checkbox ... REPOSITORY R31 Breeze REVISION DETAIL

D26572: Always render checkbox/radiobutton background

hpereiradacosta added a comment. I would strongly object to that: first that is a visual change unrelated to the issue this patch adress second this is semantically wrong, and this will break on some color theme. Things should be kept consistent. If you change the background role, you m

D26572: Always render checkbox/radiobutton background

hpereiradacosta accepted this revision. hpereiradacosta added a comment. ship it. No strong feeling. REPOSITORY R31 Breeze BRANCH checkbox-radiobutton-background (branched from master) REVISION DETAIL https://phabricator.kde.org/D26572 To: ndavis, #vdg, #breeze, #plasma, hpereiradacos

D26572: Always render checkbox/radiobutton background

hpereiradacosta added a comment. I would prefer not systematically render the background, because it might break existing applications, like rendering a widget on top of a painted image. Also this goes against Qt's rendering model which does not require rendering widget background, relying o

D26225: Change frameRadius to use pen widths, add newPenWidthFrameRadius, add PenWidth::NoPen

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. In D26225#583803 , @ndavis wrote: > - Change newPenWidthFrameRadius to frameRadiusNewPenWidth > > This sounds slightly

D26267: Replace deprecated QWeakPointer::data() with .toStrongRef().data()

hpereiradacosta added a comment. In D26267#584400 , @ndavis wrote: > In D26267#584396 , @hpereiradacosta wrote: > > > -1: > > if QWeakPointer::data() is obsoleted, (while needed in the code), I w

D26267: Replace deprecated QWeakPointer::data() with .toStrongRef().data()

hpereiradacosta added a comment. -1: if QWeakPointer::data() is obsoleted, (while needed in the code), I would object to adding toStrongRef, in between, since as pointed out by Antony, calling data immediately after brings no further security, and just results in some overhead (Weak to sha

D26225: Add newFrameRadius, change frameRadius to use pen widths, add PenWidth::NoPen

hpereiradacosta added inline comments. INLINE COMMENTS > breezehelper.h:318 > +//* frame radius with new pen width > +constexpr qreal newFrameRadius ( const qreal oldRadius, const int > penWidth ) const > +{ return qMax( oldRadius - (0.5 * penWidth), 0.0 ); } Would need

D26217: Add standard pen widths and replace hardcoded numbers

hpereiradacosta accepted this revision. hpereiradacosta added a comment. looks good. See comment about the symbol pen width change and then ship it. In D26217#583358 , @ngraham wrote: > ...And after these patches land, let's do the doxygen-fr

D26094: Add shadow rendering helper functions

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. Looks good and sensible. Very nice consolidation. Thanks ! REPOSITORY R31 Breeze BRANCH replace-hardcoded (branched from master) REVISION DETAIL https://phabricator

D19890: Reduce the indicator arrow size for press-and-hold menus in QToolButtons

hpereiradacosta added a comment. As a side remark: did you check how it looks when one selects "text under icon" or "text beside icon" for toolbar button ? REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D19890 To: hallas, #vdg, #breeze, ngraham, abetts, ndavis Cc: nda

D26001: Fix rubberband selection outline position

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. Fix is legit. Thanks ! I would avoid changing the alpha of the color though because: it makes no difference, and it is unrelated to the issue. Should be a different comm

D19890: Reduce the indicator arrow size for press-and-hold menus in QToolButtons

hpereiradacosta added a comment. In D19890#577204 , @ndavis wrote: > Ok, so there is an overlap problem, but it's quite rare. It happens when an icon uses 100% of the available space in the bottom right corner (or left with RTL, I think). > H

D25691: [KDecoration] Use QIcon::paint

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. Thanks ! REPOSITORY R113 Oxygen Theme REVISION DETAIL https://phabricator.kde.org/D25691 To: broulik, #plasma, hpereiradacosta Cc: plasma-devel, LeGast00n, The-Feren-

D25593: [kdecoration] Use QVariantAnimation instead of QPropertyAnimation

hpereiradacosta accepted this revision. hpereiradacosta added a comment. Thanks ! REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D25593 To: broulik, #vdg, hpereiradacosta, davidedmundson Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragr

D25543: Split Style & Helper into files by widget type

hpereiradacosta added a comment. Also, this change will make it pretty difficult to use git blame and git bisect in the future to track possible regressions. (to my knowledge at least). Might as well start a new repository. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.o

D25543: Split Style & Helper into files by widget type

hpereiradacosta added a comment. Also (and somewhat independently from this patch): with Qt6/KF6 being around the corner, I somewhat wonder about the utility of rewritting and hacking breeze at that time. Would this make sense to actually leave breeze (which is a rather well-working theme, w

D25543: Split Style & Helper into files by widget type

hpereiradacosta added a comment. You are missing copyright information and license in all the newly created files. On the review side: it is impossible to actually review, right ? As for the conceptual side: I fear this is addressing a non existing issue, and giving a wrong impression a

D24008: Make renderDialGroove() area match the maximum renderDialContents() area

hpereiradacosta added a comment. on the other hand, after checking that the dials keep the old appearance when "wrapping" is turned on, and since dials are rather seldom used anyway, I have no strong feeling against the change (still prefer the old look though) REPOSITORY R31 Breeze REVIS

D24008: Make renderDialGroove() area match the maximum renderDialContents() area

hpereiradacosta added a comment. Problem with the new design if you ask me is that it does not convey the information that you can roll around past the maximum as in a circle anymore. All other widget styles (except now breeze), use a circle metaphor for a dial ... Personally I think the ne

D23296: Simplify rendering of raised toolbuttons with menu

hpereiradacosta added a comment. In D23296#515696 , @ndavis wrote: > Hmm. I was thinking about using the button background of the dropdown menu for something like this mockup: F7265921: Screenshot_20190820_203213.png

D23296: Simplify rendering of raised toolbuttons with menu

hpereiradacosta created this revision. hpereiradacosta added reviewers: Breeze, ndavis. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. hpereiradacosta requested review of this revision. REVISION SUMMARY Following on https://phabricator.kde.org/D23169, it turns out one c

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

hpereiradacosta added a comment. Thanks ! REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D23169 To: ndavis, #vdg, #breeze, ngraham, hpereiradacosta Cc: hpereiradacosta, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, Z

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

hpereiradacosta added a comment. Zoom showing the issue mentionned above with the "current" patch (or master branch of breeze) F7250494: Screenshot_20190816_175237.png How it should look: F7250505: Screenshot_20190816_175628.png

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

hpereiradacosta reopened this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. F7250479: patch.diff REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D23169 To: ndavis, #vdg, #breeze, ngraha

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

hpereiradacosta added a comment. Hi Noah Thanks for the patch, however, it is not the right fix to the issue. If you use a light color scheme (like the default breeze), you will see that the shadow below the part of the button that corresponds to the arrow is darker than below the rest of

D22851: [SplitterProxy] Don't manually mapToGlobal

hpereiradacosta added a comment. Thanks for the fix ! Oxygen needs the same patch. Will you also push it there ? Hugo REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D22851 To: broulik, #plasma, hpereiradacosta, davidedmundson Cc: plasma-devel, LeGast00n, jralei

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

This revision was automatically updated to reflect the committed changes. Closed by commit R31:d6b2a3a36a1c: Remove unneeded 1 pixel margin around side panels (authored by hpereiradacosta). REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22138?vs=61228&id=61236

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

hpereiradacosta updated this revision to Diff 61228. hpereiradacosta added a comment. Simplified the patch: special case in ::pixelMetrics is in fact not needed, because it is overridden in ::frameContentsRect anyway CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22138?vs=61227&id=6

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

hpereiradacosta updated this revision to Diff 61227. hpereiradacosta added a comment. New patch, adding proper margin to avoid overlap with viewport CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22138?vs=60773&id=61227 REVISION DETAIL https://phabricator.kde.org/D22138 AFFECTED

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

hpereiradacosta added a comment. In D22138#491285 , @mart wrote: > In D22138#491173 , @hpereiradacosta wrote: > > > Indeed. This is a problem. As soon as treeviews are animated Qt makes a pixmap cop

  1   2   3   4   5   >