hpereiradacosta (Hugo Pereira Da Costa)
User

Projects

User does not belong to any projects.

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Wednesday

  • Clear sailing ahead.

User Details

User Since
Mar 30 2016, 2:05 PM (198 w, 4 d)
Availability
Available

Recent Activity

Thu, Jan 16

hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

Can the top of the scrollbar go below the column headers instead of next to them?

Like this:

Thu, Jan 16, 2:43 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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:


This is with a thin handle, but having the thick one does not really improve. The view looks like a frame inside a frame, first one being off-centered.
I have no clue how this can be improved

Thu, Jan 16, 1:37 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.
In D26655#595314, @mart wrote:

I noticed that when hovering on the scrollbar border, while the view area is not focused, the scrollbar handle is light gray (Breeze Dark, dark gray for Breeze) instead of blue. Moving the mouse just slightly more to the right turns the scrollbar handle blue.

That's the difference between thesubControlRect of the handle (that has been moved by one pixel to the right for the line) and the rectangle of the entire scrollbar.

to change this and have the handle always hitting, the subcontrolrects should stay the same and just paint the handle moved by one pixeltough the subcontrol rects not moving.
either is good, probably the latter approach looks a bit dirtier in the code but if you think is better from a design pov, it can be done

Thu, Jan 16, 1:32 PM · Plasma

Wed, Jan 15

hpereiradacosta accepted D26655: show a thin separator between view and scrollbar.

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 antialiasing. It is a minor detail, so feel free to ignore.
Other than that, no more comments !

Wed, Jan 15, 6:45 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 ?

Wed, Jan 15, 5:36 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 scrollbar groove and with respect to the line. I would revert to the old handle width and rather increase the scrollbar width by 1 pixel. Then adjust the positioning. Does that make sense ? Increasing the width of the arrows by one pixel might break pixel alignment and create inconsistencies with other places were arrows are rendered ...
Now I realize that the thin scrollbar will still be off centered (and always was so far) while it is well centered with your patch, since it is only 3 pixels while the thick one is 6 pixels ... This was not visible without the separator line. But it is going to dissapear with the other patch anyway. (otherwise one would probably need to increase its width to 4 pixels)

Wed, Jan 15, 4:54 PM · Plasma
hpereiradacosta added inline comments to D26655: show a thin separator between view and scrollbar.
Wed, Jan 15, 4:23 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 for QWidget based scrollbars, not QQC). Likely you need to increase the scrollbar width by 1, and tune the positionning of the groove+handle.

Wed, Jan 15, 3:40 PM · Plasma

Tue, Jan 14

hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 contradicting each other. It means essentially that we either don't know what we are doing, or dont think our changes enough. This is bad imo

If I had been around back then or noticed the patch, I would have made this argument.

Tue, Jan 14, 9:16 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.
In D26655#594280, @mart wrote:

Is this really intended ?
I would at least keep the color blending, making the color stronger only when the scrollbar is actually hovered.

ouch, well spotted, i will fix that :)

Tue, Jan 14, 5:13 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 handle that few people dislike.

Tue, Jan 14, 5:12 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 stronger in terms of colors, especially in the case where the parent list has focus (you get a strong blue handle).
Is this really intended ?
I would at least keep the color blending, making the color stronger only when the scrollbar is actually hovered.

Tue, Jan 14, 4:41 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.
Tue, Jan 14, 4:32 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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.

With all due respect:
This is irrelevant unless you do an actual poll.
You will have user feedback that want the thin scrollbar back. (I will).

Tue, Jan 14, 4:27 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 ?

That problem is caused by overlapping scrollbars, D26530 makes them no longer overlapping thus fixing that problem.

Tue, Jan 14, 3:24 PM · Plasma
hpereiradacosta added a comment to D26655: show a thin separator between view and scrollbar.

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 have as many complaints after the revert for people to have it back, as now of people who want the thick one. I would be among the former. Going back and forth in these design issues really gives wrong impression IMHO on our design capabilities, and I think this would be one of VDG's task to avoid this at all cost.

Now, the thin bar might not work very well with the new separator (and both pursue opposite goals: one to make the scrollbar more visible and the other less visible).
But then again, it just means that we, (including our designers and usuability experts), keep going back and forth on which direction to follow (here regarding whether we should make the scrollbars more prominent or less prominent). This does not give a good impressing to the outside world IMHO.

  • 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 ?
Tue, Jan 14, 2:45 PM · Plasma

Mon, Jan 13

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

Otherwise, it should be Window Background. Doing it that way would preserve the original look in most cases.

This would lead to some regression (I think) for unchecked checkboxes in lists. (because of window background being used).

This doesn't seem to have any visual regressions like that.

Mon, Jan 13, 9:04 PM · Plasma
hpereiradacosta added a comment to D26572: Always render checkbox/radiobutton background.

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.

Mon, Jan 13, 7:19 PM · Plasma
hpereiradacosta added a comment to D26572: Always render checkbox/radiobutton background.

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 ...
I would really revert this part of the change

To be clear, that change hasn't been made.

I'm confused: it has happened, no ? If I apply the patch I now get a white background (with normal breeze color scheme), behind checkboxes and radio button, always, which was not there before. This is , I think because of the change at line 3889.

Mon, Jan 13, 6:17 PM · Plasma
hpereiradacosta added a comment to D26572: Always render checkbox/radiobutton background.

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 ...

Mon, Jan 13, 5:09 PM · Plasma
hpereiradacosta added a comment to D26572: Always render checkbox/radiobutton background.

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 must also change the foreground role, and then you might really get funny results.

Mon, Jan 13, 5:02 PM · Plasma
hpereiradacosta accepted D26572: Always render checkbox/radiobutton background.

ship it. No strong feeling.

Mon, Jan 13, 3:11 PM · Plasma

Sat, Jan 11

hpereiradacosta added a comment to D26572: Always render checkbox/radiobutton background.

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 on that of the parent widget or window itself. I would instead either do it in qqc, or detect if we are rendering a qqc control and in this case only render the background systematically. There are several places in the code that do that already. Look of "#if BREEZE_HAVE_QTQUICK" blocks.

Sat, Jan 11, 10:40 PM · Plasma

Sat, Jan 4

hpereiradacosta committed R31:373954acd434: removed unused variables (authored by hpereiradacosta).
removed unused variables
Sat, Jan 4, 4:45 AM
hpereiradacosta committed R31:af4110dec5ca: fixed qt4 compilation (authored by hpereiradacosta).
fixed qt4 compilation
Sat, Jan 4, 4:45 AM

Mon, Dec 30

hpereiradacosta accepted D26225: Change frameRadius to use pen widths, add frameRadiusForNewPenWidth, add PenWidth::NoPen.
  • Change newPenWidthFrameRadius to frameRadiusNewPenWidth

    This sounds slightly better to me and allows it to show up next to frameRadius when autocompleting.
Mon, Dec 30, 12:12 AM · Plasma
hpereiradacosta accepted D26268: Fix QTime function deprecation warnings by switching to QElapsedTimer.
Mon, Dec 30, 12:10 AM · Plasma

Sun, Dec 29

hpereiradacosta added a comment to D26267: Replace deprecated QWeakPointer::data() with .toStrongRef().data().

-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 shared pointer). It means that either

  • weakPointer::data should not be obsoleted
  • the kdecoration api should be changed (to return shared pointers, or provide direct access to the data).

So this patch is not the correct thing to do at all, even if I make the changes @anthonyfieroni suggested. I doubt getting Qt to undeprecate QWeakPointer::data() is an option, so it seems like changing KDecoration would be the correct thing to do. Does that seem correct to you?

Sun, Dec 29, 10:42 PM · Plasma
hpereiradacosta added a comment to D26267: Replace deprecated QWeakPointer::data() with .toStrongRef().data().

-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 shared pointer). It means that either

  • weakPointer::data should not be obsoleted
  • the kdecoration api should be changed (to return shared pointers, or provide direct access to the data).
Sun, Dec 29, 10:12 PM · Plasma

Fri, Dec 27

hpereiradacosta added inline comments to D26225: Change frameRadius to use pen widths, add frameRadiusForNewPenWidth, add PenWidth::NoPen.
Fri, Dec 27, 10:42 PM · Plasma
hpereiradacosta accepted D26217: Add standard pen widths and replace hardcoded numbers.

looks good. See comment about the symbol pen width change and then ship it.

Fri, Dec 27, 10:38 PM · Plasma

Dec 19 2019

hpereiradacosta accepted D26094: Add shadow rendering helper functions.

Looks good and sensible. Very nice consolidation. Thanks !

Dec 19 2019, 3:59 PM · Plasma

Dec 17 2019

hpereiradacosta added a comment to D24281: Add default shortcut to switch to the desktop to the left/right/top/bottom.

Just a remark: WASD is very keyboard layout dependent. For AZERTY keyboards (as used in France for instance) this is completely broken, and something any gamer has to always change manually (to ZQSD) if not already done automatically by the game (which may games do)
Arrows on the other hand are much less layout dependent.

Dec 17 2019, 6:00 PM · KWin

Dec 14 2019

hpereiradacosta added a comment to D19890: Reduce the indicator arrow size for press-and-hold menus in QToolButtons.

As a side remark: did you check how it looks when one selects "text under icon" or "text beside icon" for toolbar button ?

Dec 14 2019, 7:31 PM · Plasma
hpereiradacosta accepted D26001: Fix rubberband selection outline position.

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 commit (which you can do without review if you really think it is important)

Dec 14 2019, 7:28 PM · VDG, Breeze, Plasma

Dec 13 2019

hpereiradacosta added a comment to D19890: Reduce the indicator arrow size for press-and-hold menus in QToolButtons.

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).
Here I changed the stop icon in KDevelop to the icon for Codelite:

.

I could get around this in a number of ways that all have their downsides:
1/ Move the arrow to the very bottom right and make sure it's small enough that no clipping happens

  • This looks bad and makes it harder to see the arrow
Dec 13 2019, 10:31 PM · Plasma

Dec 9 2019

hpereiradacosta added a comment to D25814: [KColorScheme] Add SeparatorColor.

I'm not sure what you mean when you wonder how it would play with monochrome icons. It should have nothing to do with them unless you embed stylesheets in the SVGs to use the separator color in breeze icons.

Dec 9 2019, 8:20 PM · Frameworks
hpereiradacosta added a comment to D25814: [KColorScheme] Add SeparatorColor.

Can we clarify what separators we're referring to.

Frames around UI elements and horizontal/vertical lines that separate UI elements.
These are the most common examples, but probably not absolutely everything:

Dec 9 2019, 4:19 PM · Frameworks
hpereiradacosta added a comment to D25814: [KColorScheme] Add SeparatorColor.

So the background here is that we've gotten a new VDG designer who produced mockups of Breeze dark with dark separators, and some people absolutely fell in love with them, while other people hated them. We could not achieve consensus on moving to use dark separators for Breeze Dark, so there was a desire to offer people that choice. It occurs to me that we could put this in the Breeze style's own settings, if it's strictly necessary to expose this to users. Personally I would prefer to just make a decision on separator colors one way or the other rather than making it explicitly configurable (dark separators FTW :) ).

Dec 9 2019, 4:10 PM · Frameworks
hpereiradacosta added a comment to D25814: [KColorScheme] Add SeparatorColor.

Few more comments on this:

  • general: you will never be able to make all the opiniated people happy, and you have to draw a line (otherwise your code will become bloated, buggy, and unmaintainable)
  • regarding this specific case: many widget style will not implement this new color. For those this will just generate bugs reports: why is my color scheme not respectd ?
  • some widget styles (oxygen at least, but I'm sure there are others) use two colors for frames and separators, to mimic shadows or relief effects. Adding one single color for this will not fit such schemes.
  • in the end if you need an extra color for a given theme (be it future-breeze or whatever), there is also the possibility to add it as a extra option for this specific theme, rather than forcing it to kcolorscheme and imposing it to all styles (or making kcolorscheme broken for all the styles that wont use it).

This is how window decoration shadows and glow were handled to oxygen.
I think this change will break more things than it will fix, especially if the fix is to make some opinionated people happy.

Dec 9 2019, 5:36 AM · Frameworks
hpereiradacosta updated subscribers of D25814: [KColorScheme] Add SeparatorColor.

Would also be good to have the opinion of @cfeck on this.

Dec 9 2019, 1:00 AM · Frameworks
hpereiradacosta added a comment to D25814: [KColorScheme] Add SeparatorColor.

Adding new entries to the kcolorscheme should be done with a lot of care, because it could be seen as some sort of API break for existing colorscheme, as soon as you start using this color in the widget style: you would need a fallback implementation, for all the colorscheme in the open which do not implement this particular color. These kind of additions happened a lot whith gtk3 css style and resulting of overall unhappiness and distrust from theme/color scheme developpers. This also increases the complexity of the code and difficulty to maintain.

Dec 9 2019, 12:29 AM · Frameworks

Dec 2 2019

hpereiradacosta accepted D25691: [KDecoration] Use QIcon::paint.

Thanks !

Dec 2 2019, 6:52 PM · Plasma

Nov 28 2019

hpereiradacosta accepted D25593: [kdecoration] Use QVariantAnimation instead of QPropertyAnimation.

Thanks !

Nov 28 2019, 7:35 PM · Plasma

Nov 27 2019

hpereiradacosta added a comment to D25543: Split Style & Helper into files by widget type.

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.

Nov 27 2019, 4:12 AM · Plasma
hpereiradacosta added a comment to D25543: Split Style & Helper into files by widget type.

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, with very little bug reports) unchanged, and start working anew on a new widget style targeting KF6 ? Would that not be more exciting, bring more people in, trigger new ideas, give more freedom ? And be a more ambitious task all in all ?
One could of course start from breeze, and apply all the new ideas about how the code should be organized, how highlight should look, and checkboxes, without disturbance for something which has lived largely unchanged (and with not so many complains) for all the kf5 lifetime ...

Nov 27 2019, 3:53 AM · Plasma
hpereiradacosta added a comment to D25543: Split Style & Helper into files by widget type.

You are missing copyright information and license in all the newly created files.
On the review side: it is impossible to actually review, right ?

Nov 27 2019, 3:41 AM · Plasma

Nov 13 2019

hpereiradacosta updated subscribers of T12027: Relicence breeze to LGPL.

The question was already raised in the past. At the time there strong objections against this, some of them raised by Martin (now added as subscriber) and to which I agreed, but which I can't remember today. @graesslin any recollection ?

Nov 13 2019, 7:50 PM · Breeze, Goal: All About the Apps

Oct 7 2019

hpereiradacosta committed R31:0de811bc2505: Fix color of dropdown arrow for focused toolbuttons with long-press menus. (authored by hpereiradacosta).
Fix color of dropdown arrow for focused toolbuttons with long-press menus.
Oct 7 2019, 9:10 PM

Sep 18 2019

hpereiradacosta added a comment to D24008: Make renderDialGroove() area match the maximum renderDialContents() area.

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)

Sep 18 2019, 8:16 PM · Plasma
hpereiradacosta added a comment to D24008: Make renderDialGroove() area match the maximum renderDialContents() area.

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 new look just looks ... broken. It looks like a bent scrollbar, (or a bent slider), which a dial is not ...

Sep 18 2019, 7:56 PM · Plasma

Aug 28 2019

hpereiradacosta added a comment to D23415: Improve comprehensibility and consistency of window placement mode names.

for what its worth: to me smart means: there has been some thought put in this placement policy and it should be the best one for you. If you don't like it, you might want to try the alternatives. When I read "smart" as an option, immediately I think "that's what I want", and select it, if not already selected. In other words, it is another (better) way of saying "default", without having to go through the actual description of what it means. And then of course, I must trust the devs that "smart" is indeed smart, and is indeed what I should want, even if I don't understand it.

Aug 28 2019, 7:40 PM · Documentation, KWin

Aug 21 2019

hpereiradacosta abandoned D23296: Simplify rendering of raised toolbuttons with menu.
Aug 21 2019, 12:47 PM · Plasma
hpereiradacosta added a comment to D23296: Simplify rendering of raised toolbuttons with menu.

Hmm. I was thinking about using the button background of the dropdown menu for something like this mockup:

Aug 21 2019, 8:06 AM · Plasma

Aug 20 2019

hpereiradacosta updated the summary of D23296: Simplify rendering of raised toolbuttons with menu.
Aug 20 2019, 3:43 PM · Plasma
hpereiradacosta requested review of D23296: Simplify rendering of raised toolbuttons with menu.
Aug 20 2019, 3:42 PM · Plasma

Aug 16 2019

hpereiradacosta added a comment to D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu.

Thanks !

Aug 16 2019, 7:04 PM · Plasma
hpereiradacosta added a comment to D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu.

Zoom showing the issue mentionned above with the "current" patch (or master branch of breeze)

Aug 16 2019, 3:56 PM · Plasma
hpereiradacosta requested changes to D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu.
Aug 16 2019, 3:50 PM · Plasma
hpereiradacosta reopened D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu.

Aug 16 2019, 3:50 PM · Plasma
hpereiradacosta added a comment to D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu.

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 the button. This is because the frame is actually rendered twice.

Aug 16 2019, 3:46 PM · Plasma

Aug 6 2019

hpereiradacosta committed R31:c92416356653: Removed GlobalPos from QMouseEvent constructor (authored by hpereiradacosta).
Removed GlobalPos from QMouseEvent constructor
Aug 6 2019, 12:07 PM

Aug 5 2019

hpereiradacosta added a comment to D22884: [RFC] Don't show title on page by default.

"KPageWidgetItems live in a multi-page KPageDialog"
Is that really true ? Does it not live also standalone when running e.g. kcmshell5 kwindecoration (or any other kcm ?)
If yes, and at least in this case, the title has to stay, because there is no side bar.

Aug 5 2019, 12:49 PM · Frameworks

Aug 1 2019

hpereiradacosta added a comment to D22851: [SplitterProxy] Don't manually mapToGlobal.

Thanks for the fix !
Oxygen needs the same patch. Will you also push it there ?

Aug 1 2019, 7:11 PM · Plasma

Jul 26 2019

hpereiradacosta added a comment to T11124: Unify highlight effect style.
In T11124#193005, @mglb wrote:

@ndavis Install event filter on qGuiApp, handle QEvent::ApplicationPaletteChange and call configurationChanged. Should work.

Hover/focus color change is not handled because there is QApplication::setPalette and QGuiApplication::setPalette. The former is used by KColorSchemeManager for changing color scheme and does not emit paletteChanged signal, which is used to detect the change in the style.

Jul 26 2019, 8:10 PM · Plasma, Breeze, Goal: Consistency, VDG

Jul 25 2019

hpereiradacosta committed R113:f2b5bb942fb2: removed duplicated code (oxygenitemmodel is already in liboxygen) This fixes a… (authored by hpereiradacosta).
removed duplicated code (oxygenitemmodel is already in liboxygen) This fixes a…
Jul 25 2019, 8:41 PM

Jul 22 2019

hpereiradacosta committed R113:ce3b0622915e: Moved more functions from protected to private (authored by hpereiradacosta).
Moved more functions from protected to private
Jul 22 2019, 11:50 AM
hpereiradacosta committed R113:d2bbf1c95c0d: Fixed deprecation warnings Fixed kde4 compilation (authored by hpereiradacosta).
Fixed deprecation warnings Fixed kde4 compilation
Jul 22 2019, 11:50 AM
hpereiradacosta committed R113:03ba45add3f7: Fixed override warnings Removed unnecessary virtuals Moved some protected… (authored by hpereiradacosta).
Fixed override warnings Removed unnecessary virtuals Moved some protected…
Jul 22 2019, 11:50 AM

Jul 20 2019

hpereiradacosta added a comment to D22083: introduce concept of header and footer for kpageview.

Hi Marco,
Patch works like a charm in standard config dialogs.
As a somewhat off topic remark though, it does not work in kcmultidialog (in framework kcmutils), even though it derives from kpageddialog.
This shows up for instance in the "window manager settings" you can get from the decoration menu (Alt+F3), or simply when running oxygen-setting or breeze-settings.
Looking into kcmultidialog, it seems there is some internal handling of margins in there, for a reason unknown to me. Maybe you want to investigate there too (in a different patch)

Jul 20 2019, 1:36 PM · Frameworks

Jul 19 2019

hpereiradacosta committed R113:aea26bdbac0a: fixed layout margin for consistency with other pages (authored by hpereiradacosta).
fixed layout margin for consistency with other pages
Jul 19 2019, 11:14 AM
hpereiradacosta committed R113:8c670cad0415: - use kPageDialog for oxygen-demo - moved unneeded protected virtual… (authored by hpereiradacosta).
- use kPageDialog for oxygen-demo - moved unneeded protected virtual…
Jul 19 2019, 11:14 AM

Jul 15 2019

hpereiradacosta added a comment to T11124: Unify highlight effect style.

Can I change the selection background color and use the selection color for the background of highlights? Then the regular highlight color could be used mainly for line highlights and outlines.

Jul 15 2019, 1:49 PM · Plasma, Breeze, Goal: Consistency, VDG
hpereiradacosta added a comment to T11124: Unify highlight effect style.

Thanks for the color overview. I'll avoid changing the highlighted text color in the colorscheme.

Personal opinion follows.

In my personal opinion, the current breeze widget style highlight model is fine as it is, internally consistent, and consistent with the breeze visual goal. It is simple (in particular in lists and menus), it minimizes the use of frames, and is also consistent with text selection (something on which you have no handle). I also note that it is quite trendy, when comparing e.g. to highlight model of many web-based applications (slack, github, etc.), or to the other widget styles shipped by Qt.
To ensure consistency with plasma, rather than modifying the complex beast that is the widget style highlight model, I would have changed the plasma model instead. Simpler, both implementation-wise and visually wise.
My two cents.

That's a fair point, but the current style does have rather poor contrast with light text. While dark text has significantly better contrast, I don't like the way it looks against Plasma Blue. The highlight color could be changed, which would be much easier than changing the highlight style, but that would affect everything that currently uses the highlight color, including icons.

Jul 15 2019, 12:52 PM · Plasma, Breeze, Goal: Consistency, VDG
hpereiradacosta added a comment to T11124: Unify highlight effect style.

Personal opinion follows.

Jul 15 2019, 12:09 PM · Plasma, Breeze, Goal: Consistency, VDG
hpereiradacosta added a comment to T11124: Unify highlight effect style.

Few words about colors (sorry if you know all about it already)

Jul 15 2019, 12:01 PM · Plasma, Breeze, Goal: Consistency, VDG

Jul 14 2019

hpereiradacosta added a comment to T11124: Unify highlight effect style.
  • For the record: toolbar buttons (or rather QToolButtons) can have focus too, when they are used outside of toolbars (and there are examples of such everywhere, for instance when you have an "open" button next to a text entry for selecting a file.

your enumeration did not mention

  • checkboxes and radiobuttons who also have hove/focus + the various selected states.
  • tabbar entries (hover, focus, selected)
  • toolboxes
Jul 14 2019, 1:39 PM · Plasma, Breeze, Goal: Consistency, VDG

Jul 5 2019

hpereiradacosta committed R31:d6b2a3a36a1c: Remove unneeded 1 pixel margin around side panels (authored by hpereiradacosta).
Remove unneeded 1 pixel margin around side panels
Jul 5 2019, 5:59 PM
hpereiradacosta closed D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.
Jul 5 2019, 5:59 PM · Plasma
hpereiradacosta updated the diff for D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.

Simplified the patch: special case in ::pixelMetrics is in fact not needed, because it is overridden in ::frameContentsRect anyway

Jul 5 2019, 3:50 PM · Plasma
hpereiradacosta added a comment to D22083: introduce concept of header and footer for kpageview.
In D22083#491299, @mart wrote:

code issues are fixed.
Now for removing the further margin around the whole window:
should that be done here or in breeze?
if done here, it may make it look bad on all other widget styles?

Jul 5 2019, 3:39 PM · Frameworks
hpereiradacosta updated the summary of D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.
Jul 5 2019, 3:35 PM · Plasma
hpereiradacosta updated the diff for D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.

New patch, adding proper margin to avoid overlap with viewport

Jul 5 2019, 3:34 PM · Plasma
hpereiradacosta added a comment to D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.
In D22138#491285, @mart wrote:

Indeed. This is a problem. As soon as treeviews are animated Qt makes a pixmap copy of the treeview viewport. This does not include the blue line on the side, which is painted in the frame, below the (transparent) viewport. Will investigate further.

would still adding a pixel on the right work around the issue?

Jul 5 2019, 3:21 PM · Plasma
hpereiradacosta added a comment to D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.

Indeed. This is a problem. As soon as treeviews are animated Qt makes a pixmap copy of the treeview viewport. This does not include the blue line on the side, which is painted in the frame, below the (transparent) viewport. Will investigate further.

Jul 5 2019, 7:27 AM · Plasma

Jul 4 2019

hpereiradacosta added a comment to D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.

Very nice! In conjunction with Marco's patch (D22083), I now see the following for Dolphin's settings window:

I notice that your screenshot depicts the sidebar with no top, bottom, or left margins, which is the indended appearance. Is that the result of some other required patch I haven't applied yet, or did I do something wrong?

Jul 4 2019, 6:46 PM · Plasma
hpereiradacosta added a comment to D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.

Since this apparently doesn't need review,

This is not what I said. What I said is "thanks, it looks fantastic" is not a proper review and hence irrelevant. What I expect for a proper review is

  • does the patch achieve the behavior matching its title, and is it the intended behavior
  • does the code look legit (compiles, properly formatted, etc.)
  • is there a better way to achieve the same
  • does the patch break other things, and how can it be improved so that it doesn't

If you can provide such a review I'd be happy to modify the patch accordingly.

Jul 4 2019, 6:44 PM · Plasma

Jul 1 2019

hpereiradacosta added a comment to D22083: introduce concept of header and footer for kpageview.

Hi Marco,
see https://phabricator.kde.org/D22138
(I also included the white background in there, because it turned out a bit tricky).

Jul 1 2019, 11:02 AM · Frameworks

Jun 28 2019

hpereiradacosta added a comment to D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.

Thanks @hpereiradacosta! This looks fantastic. Adding @ndavis and @filipf for comment since they've been working on this project too from other angles.

Jun 28 2019, 2:37 PM · Plasma
hpereiradacosta committed R31:62ac0c1820de: - fixed "missing override" warnings - removed useless "virtual" specifications… (authored by hpereiradacosta).
- fixed "missing override" warnings - removed useless "virtual" specifications…
Jun 28 2019, 12:49 PM
hpereiradacosta requested review of D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background.
Jun 28 2019, 12:29 PM · Plasma
hpereiradacosta added a comment to T11124: Unify highlight effect style.
Jun 28 2019, 11:24 AM · Plasma, Breeze, Goal: Consistency, VDG
hpereiradacosta added a comment to D22083: introduce concept of header and footer for kpageview.

ather simple and attached. Feel free to add this or something similar to any other modification you plan to do.

Thank you very much Hugo.. that pixel was driving me mad ;)
Do you want to push it yourself or I just include it in the breeze modifications I'll do for the sidebar style?

Jun 28 2019, 9:25 AM · Frameworks

Jun 27 2019

hpereiradacosta added a comment to D22083: introduce concept of header and footer for kpageview.

On the breeze side, asside from the layout margins/spacing issues, there is at least one hard-coded pixel in the frame rendering that must be removed. In fact I have this committed in my local breeze version already. The corresponding patch is rather simple and attached. Feel free to add this or something similar to any other modification you plan to do.

Jun 27 2019, 2:49 PM · Frameworks

Jun 7 2019

hpereiradacosta added a comment to D13481: Recommend window border size "None".
Jun 7 2019, 6:01 AM · Plasma

Jun 3 2019

hpereiradacosta committed R113:72a70ceacc04: Disable extended resize borders for maximized windows CCBUG: 407989 (authored by hpereiradacosta).
Disable extended resize borders for maximized windows CCBUG: 407989
Jun 3 2019, 10:39 AM
hpereiradacosta committed R31:c95b7652b7af: Disable extended resize borders for maximized windows CCBUG: 407989 (authored by hpereiradacosta).
Disable extended resize borders for maximized windows CCBUG: 407989
Jun 3 2019, 10:26 AM

May 14 2019

hpereiradacosta added a comment to D13284: [decorations] Let KDecoration plugins recommend a border size per default.

So, how does that work when user select a specific size, and then change decoration ? Is his choice erased by the default of the new decoration ? Or is his choice maintained (in which case he never gets a chance to actually see the "preferred" width of the new decoration ? And then, if erased by the new default, does that mean that if I switch from decoration A (with custom setting) to decoration B, and then back to A, my custom setting is lost ?

Best,

Hugo

Default is to use the recommended sizes of decorations (falls back to Normal if decoration does not recommend one). User might opt in to specifiy a border size manually instead. This choice is being remembered even if he changes to another decoration. But in this configuration screen he can also opt out to automatic border size again.

May 14 2019, 5:12 AM · KWin

May 13 2019

hpereiradacosta added a comment to D13284: [decorations] Let KDecoration plugins recommend a border size per default.

So, how does that work when user select a specific size, and then change decoration ? Is his choice erased by the default of the new decoration ? Or is his choice maintained (in which case he never gets a chance to actually see the "preferred" width of the new decoration ? And then, if erased by the new default, does that mean that if I switch from decoration A (with custom setting) to decoration B, and then back to A, my custom setting is lost ?

May 13 2019, 6:47 PM · KWin

Apr 3 2019

hpereiradacosta added a comment to D19890: Reduce the indicator arrow size for press-and-hold menus in QToolButtons.

Did anyone check how this patch look with other icons than those in the screenshot ?
E.g. the preview icon in dolphin, or list sorting or search icons ? Does the new arrow overlap with the said icons ?

Apr 3 2019, 3:44 PM · Plasma