Re: KTitleWidget and the native Mac style

2017-07-05 Thread René J . V . Bertin
On Wednesday July 05 2017 10:58:59 Hugo Pereira Da Costa wrote:

>No
>the default is false (no frame):
>
>breeze.kcfg:
>
>
>
>false
>
>

Curious, I've always seen Breeze display the frame, and never activated the 
option as far as I can remember. I did notice yesterday that breezerc doesn't 
contain the key at all if you deactivate the option, so it's a bit of a mystery 
how it got set on my end. Maybe the default hasn't always been off?

>> Do you have any idea why moving the QFrame shape and background role 
>> configuration from the KTitleWidget ctor to Style::polish() has this subtle 
>> effect, or why the frame outline is NOT drawn with rounded corners in the 
>> current code?
>nope. but no big deal if there is no frame drawn anyway, right ?

True, but that's assuming you'll get away with removing the feature to draw the 
frame completely. I don't particularly care for it myself I can see how one 
could get to appreciate it :)
Actually, the issue is more "why is the frame ever drawn without rounded 
corners in the current code".

R.


Re: KTitleWidget and the native Mac style

2017-07-05 Thread René J . V . Bertin
On Wednesday July 05 2017 09:55:27 Hugo Pereira Da Costa wrote:

(CC'ing the plasma-devel ML and thus keeping Hugo's full reply as context.)

>On 07/04/2017 11:13 PM, René J.V. Bertin wrote:
>> On Tuesday July 04 2017 20:16:55 Sebastian Kügler wrote:
>>
>> @Kevin: should we continue to CC you?
>>
>>> The frame in my understanding is old weight, and can go (do check with the 
>>> VDG
>>> though!)
>> Thing is that you cannot simply get rid of the QFrame, it would at least 
>> have to be replaced with something invisible that can take over its other 
>> role.
>>
>> What works (mostly) is something like this:
>>
>> KTitleWidget::KTitleWidget(QWidget *parent)
>>  : QWidget(parent),
>>d(new Private(this))
>> {
>>  QFrame *titleFrame = new QFrame(this);
>> // titleFrame->setAutoFillBackground(true);
>> // titleFrame->setFrameShape(QFrame::StyledPanel);
>>  titleFrame->setFrameShadow(QFrame::Plain);
>> // titleFrame->setBackgroundRole(QPalette::Base);
>>
>>  // default image / text part start
>>  d->headerLayout = new QGridLayout(titleFrame);
>>  d->headerLayout->setColumnStretch(0, 1);
>> // d->headerLayout->setMargin(6);
>>  d->headerLayout->setContentsMargins(0, 0, 0, 0);
>> //...
>> }
>>
>> and replace `d->textLabel->setStyleSheet(d->textStyleSheet())` with
>>
>> d->textLabel->setFont(QFontDatabase::systemFont(QFontDatabase::TitleFont));
>>
>> (that will use the window titlebar font, but also when no platform theme 
>> plugin -aka plasma-integration- is installed).
>>
>> in Breeze we can then do
>>
>>  } else if( qobject_cast( widget ) && widget->parent() && 
>> widget->parent()->inherits( "KTitleWidget" ) ) {
>>
>>  QFrame *frame = qobject_cast( widget );
>>  if( StyleConfigData::titleWidgetDrawFrame() ) {
>>  frame->setAutoFillBackground( true );
>>  frame->setFrameShape( QFrame::StyledPanel );
>>  frame->setBackgroundRole( QPalette::Base );
>>  // make the title frame a bit less luxuriously big, and centre 
>> the text vertically
>>  frame->layout()->setContentsMargins(3, 3, 3, 0);
>>  } else {
>>  frame->setAutoFillBackground( false );
>>  frame->setFrameShape( QFrame::NoFrame );
>>  frame->setBackgroundRole( QPalette::Window );
>>  // don't take extra space for a frame we're not showing at all
>>  frame->layout()->setContentsMargins(0, 0, 0, 0);
>>  }
>>
>>  }
>Concerning the change in breeze:
>1/ I would gladly get rid of the option to draw the frame at all (not 
>sure anyone uses this anyway)

Given that it's the default I have a hunch many people actually do (if you can 
call that "using" ;)). What you can do is start by not drawing the frame by 
default, possibly disable the option to enable drawing it in breeze-settings5 
and see what kind of feedback you get on that.
Do you have any idea why moving the QFrame shape and background role 
configuration from the KTitleWidget ctor to Style::polish() has this subtle 
effect, or why the frame outline is NOT drawn with rounded corners in the 
current code?

>2/ however, the default layout should not change with respect to what we 
>have now, unless blessed by the vdg and plasma team.
>That implies:
>- keep the current font size increment
>- don't change the margins.

Was the font increment blessed by either when Sebas introduced it, or maybe 
just the general idea of using a larger font?
I've CC'ed the plasma-devel ML, maybe you (Hugo) can loop in (someone from) the 
vdg team?

This also implies that appropriate display of the KTitleWidget on Mac (and MS 
Windows?) will require testing for the style in use in KWidgetsAddons.

R.


Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin
On Tuesday July 04 2017 20:16:55 Sebastian Kügler wrote:

>The frame in my understanding is old weight, and can go (do check with the VDG 

I noticed it was already there in KDE4, indeed, though not usually rendered.

>but to me, more importantly, reducing its size will 
>lead to regressions in Plasma. 

Avoiding regressions is evident, but how could changing the size lead to 
regressions in properly written code (which should be able to handle changes in 
font weight, style or size)?

>I have no problems with moving its rendering to the style so it can depend on 
>the platform used, or another technically sound solution, but a change should 
>not introduce regressions in Plasma.

Note that I wasn't particularly concerned with the size, but I did think about 
this. I think widget styles can influence the size of the font used in QLabels 
but that could be tricky because it affects the widget size. There are 
typically more QLabels in a view than QFrames which may make runtime detection 
of KTitleWidget labels (through inheritance) a bit expensive.

Adding a font role (or repurposing the window titlebar font role) would be the 
most logical approach, but not easily deployed.

Maybe we could simply use QFontDatabase::SystemFont::TitleFont instead of 
imposing a hardcoded size increment?

R.


Re: KTitleWidget and the native Mac style

2017-07-04 Thread Sebastian Kügler
On dinsdag 4 juli 2017 17:55:01 CEST René J.V. Bertin wrote:
> Me neither for myself, I'm happy with how my QtCurve theme looks but I'm
> also trying to improve the way KDE software looks using the native style.

The frame in my understanding is old weight, and can go (do check with the VDG 
though!), but the larger font isn't. It's semantically a sane choice (titles 
often have larger fonts), but to me, more importantly, reducing its size will 
lead to regressions in Plasma. 

I have no problems with moving its rendering to the style so it can depend on 
the platform used, or another technically sound solution, but a change should 
not introduce regressions in Plasma.

Cheers,
-- 
sebas

http://www.kde.org | http://vizZzion.org


Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin
Hi,

How about this?

In KWidgetAddons (out-commented code shows "stock" 5.35.0):

KTitleWidget::KTitleWidget(QWidget *parent)
: QWidget(parent),
  d(new Private(this))
{
QFrame *titleFrame = new QFrame(this);
// titleFrame->setAutoFillBackground(true);
// titleFrame->setFrameShape(QFrame::StyledPanel);
titleFrame->setFrameShadow(QFrame::Plain);
// titleFrame->setBackgroundRole(QPalette::Base);

In breeze/kstyle/breezestyle.cpp (Style::polish(QWidget*)):

} else if( qobject_cast( widget ) && widget->parent() && 
widget->parent()->inherits( "KTitleWidget" ) ) {

// widget->setAutoFillBackground( false );
// if( !StyleConfigData::titleWidgetDrawFrame() )
// { widget->setBackgroundRole( QPalette::Window ); }
QFrame *frame = qobject_cast( widget );
if( StyleConfigData::titleWidgetDrawFrame() ) {
frame->setAutoFillBackground( true );
frame->setFrameShape( QFrame::StyledPanel );
frame->setBackgroundRole( QPalette::Base );
} else {
frame->setAutoFillBackground( false );
frame->setFrameShape( QFrame::NoFrame );
frame->setBackgroundRole( QPalette::Window );
}

}

That moves the entire choice of the title frame rendering style to the style 
plugin and the end result looks the same (I didn't to a pixel analysis yet).

R.



Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin
On Tuesday July 04 2017 18:02:13 Hugo Pereira Da Costa wrote:

> One should really consult with vdg here, since all these points (larger 
> fonts, padding, no frame), was already discussed with them, and agreed 
> upon, back in the days.

As far as the official 1 or 2 KDE styles are concerned, I presume (hope).

R.


Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin
On Tuesday July 04 2017 15:49:28 Kevin Funk wrote:

>With regards to the "Widget Style and Behaviour" label in the `kcmshell5 
>style` dialog: I don't find that particularly attracting under Breeze as well.
>
>It's too "bulky" for my taste. 

Indeed. I just notice that contrary to what you'd expect, unticking "draw a 
frame around page titles" in the Breeze settings only makes the frame invisible 
but does NOT remove the space allocated for it. It would probably look a lot 
less bulky without that useless padding, or if it adopted the same height as 
the text to look more like a banner.

I agree it isn't particularly necessary to use a bigger font, but that too is a 
bit subject to personal taste. It would probably have made more sense to 
introduce a new font role/category for this (or repurpose the Window Title font 
role).

>Maybe we can find a solution that works well under both styles?

Hmmm, Breeze already checks if widgets inherit KTitleWidget in order to decide 
if it must apply the aforementioned setting 
(StyleConfigData::titleWidgetDrawFrame()). That means one could obtain the same 
result as far as the frame is concerned even if KTitleWidget does NOT create a 
filled and outlined frame itself. The QtCurve style also checks for 
KTitleWidgets (styles shipped by Qt evidently won't do that).

IOW, KTitleWidget only needs to create a transparent/invisible QFrame. Breeze 
and QtCurve can decide whether and how to render it much like they already do; 
Oxygen already skips renders it invisibly.

Alternatively the KTitleWidget ctor could try to read the TitleWidgetDrawFrame 
key from breezerc with a default of False. That should cover things on 
non-Plasma DEs, but it's not very elegant.

I don't think the platform theme plugin can be of real influence here, can it? 
FWIW, I do have a standalone fork of the Macintosh widget style (from Qt 5.9, 
works with 5.8 too) in which I could add KDE-specific code but that would 
benefit only those applications that embed the plugin.

>This patch removes the bold weight from KTitleWidget and makes the text
> a bit bigger, improving focus. This is more in line with common
> expectations of a title, and it's more in line with Plasma 5's
> typography.

I'm not sure what my common expectations are for a dialog title but I think 
they'd put the title text in the window titlebar. Or else centred horizontally, 
possibly with underlining. Maybe a style could underline text over the entire 
widget width? Both (centring and underlining) would not look as much deviant on 
Mac as the current title look.

FWIW and beyond the topic at hand: Mac dialogs that adopt a tabbed design 
usually have the tabs (icons or text labels) in a horizontal top row so they 
already have something like a title at the top. If I were to add a title to 
such dialogs I'd experiment with putting it in a completely different position, 
at the lower right, above or under the OK/Cancel buttons. That's often where 
you look first when a dialog opens, and also the last region (if you favour the 
mouse over the keyboard).

>PS: I've no incentive for getting this fixed, just wanted to add my 2c and 
>connect people.

Me neither for myself, I'm happy with how my QtCurve theme looks but I'm also 
trying to improve the way KDE software looks using the native style.

R.


Re: KTitleWidget and the native Mac style

2017-07-04 Thread Kevin Funk
On Tuesday, 4 July 2017 14:53:15 CEST René J. V. Bertin wrote:
> René J.V. Bertin wrote:
> > style. I think I figured out the how/where once but can't seem to find the
> > info anymore so I'd appreciate a pointer.
> 
> Found it.
> 
> > FWIW, drawing this kind of label in a frame and/or with a different
> > background colour isn't appropriate for the native Macintosh style.
> 
> And this is where we have a problem if you're adamant about being as much in
> line with native look-and-feel as possible, as I know some KDE devs are.
> 
> I haven't found a single example of a "native" Mac application that uses
> comparable page/dialog titles rendered with a bigger font and in a frame
> with a different background colour.
> 
> I myself don't really care one way or another, if anything I find it more
> important that the user has a choice at some level. Ideally this would go
> through the widget style (as Breeze allows and QtCurve probably too) but the
> Macintosh style is not configurable and I don't think Qt would consider
> adding the rather complex patch required to render KTitleWidgets
> appropriately.
> 
> That leaves us with the only option of modifying the KTitleWidget code.

With regards to the "Widget Style and Behaviour" label in the `kcmshell5 
style` dialog: I don't find that particularly attracting under Breeze as well.

It's too "bulky" for my taste. 

CC'ing Sebas, who've reworked the widget to look like this in

commit 510236aa9cea6bae48e548b42b5b721195bed121
Author: Sebastian Kügler 
Date:   Sat May 24 01:36:51 2014 +0200

Change titlewidget from bold to increased font size

This patch removes the bold weight from KTitleWidget and makes the text
a bit bigger, improving focus. This is more in line with common
expectations of a title, and it's more in line with Plasma 5's
typography.


Maybe we can find a solution that works well under both styles?

PS: I've no incentive for getting this fixed, just wanted to add my 2c and 
connect people.

Cheers,
Kevin

> I'll
> submit a patch for review that adds a runtime check of the widget style in
> use but if anyone has a better idea I wouldn't mind hearing it and avoiding
> unnecessary work.
> 
> R.


-- 
Kevin Funk | kf...@kde.org | http://kfunk.org

signature.asc
Description: This is a digitally signed message part.


Re: KTitleWidget and the native Mac style

2017-07-04 Thread René J . V . Bertin
René J.V. Bertin wrote:

> style. I think I figured out the how/where once but can't seem to find the
> info anymore so I'd appreciate a pointer.

Found it.

> FWIW, drawing this kind of label in a frame and/or with a different background
> colour isn't appropriate for the native Macintosh style.

And this is where we have a problem if you're adamant about being as much in 
line with native look-and-feel as possible, as I know some KDE devs are.

I haven't found a single example of a "native" Mac application that uses 
comparable page/dialog titles rendered with a bigger font and in a frame with a 
different background colour.

I myself don't really care one way or another, if anything I find it more 
important that the user has a choice at some level. Ideally this would go 
through the widget style (as Breeze allows and QtCurve probably too) but the 
Macintosh style is not configurable and I don't think Qt would consider adding 
the rather complex patch required to render KTitleWidgets appropriately.

That leaves us with the only option of modifying the KTitleWidget code. I'll 
submit a patch for review that adds a runtime check of the widget style in use 
but if anyone has a better idea I wouldn't mind hearing it and avoiding 
unnecessary work.

R.