D6518: guard against themes without a valid shadow
anthonyfieroni added inline comments.
INLINE COMMENTS
> dialogshadows.cpp:270
> {
> m_shadowPixmaps << q->pixmap(element);
> }
Validate element (hasElement and > 0) or add default pixmap.
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D6518
To: mart, #plasma
Cc: anthonyfieroni, davidedmundson, plasma-devel, #frameworks, ZrenBot,
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart,
lukas
D6518: guard against themes without a valid shadow
davidedmundson added a comment. What if a theme only has shadow-top, but not right etc. (or a more likely scenario, they exist, but the metadata has them be 0px big) What would happen if you have a theme with shadow then switch to one without? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D6518 To: mart, #plasma Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6519: Use CMAKE_INSTALL_BINDIR for dbus service generation
bkchr created this revision. bkchr added a project: KSecrets Service. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY The old implementation used CMAKE_INSTALL_PREFIX for determining the installation directory. The target is installed by using KF5_INSTALL_TARGETS_DEFAULT_ARGS and that internally uses CMAKE_INSTALL_BINDIR. In my case I used a distribution (Nixos) where CMAKE_INSTALL_BINDIR is not a sub directory of CMAKE_INSTALL_PREFIX and thus Dbus could not find the kwallet5d executable. REPOSITORY R311 KWallet REVISION DETAIL https://phabricator.kde.org/D6519 AFFECTED FILES src/runtime/kwalletd/org.kde.kwalletd5.service.in To: bkchr Cc: #frameworks
D6518: guard against themes without a valid shadow
mart created this revision. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY some themes don't have a valid shadow, which would lead to a crash in kwayland, don't even try to render a shadow if one of the elements is missing BUG:381953 TEST PLAN Doesn't crash anymore, i can't hellp but think something should be fixed in kwayland as well REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D6518 AFFECTED FILES src/plasmaquick/dialogshadows.cpp To: mart, #plasma Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6331: Make sure that the tsfiles target is generated
ltoscano accepted this revision. ltoscano added a comment. The issue initially reported is fixed. I tried with //fr// and other 3 languages which should be representative enough (//de//, //uk//, //sr@ijekavian//; the latter was complaining before the last revision of the review). REPOSITORY R249 KI18n REVISION DETAIL https://phabricator.kde.org/D6331 To: apol, #frameworks, ltoscano, lueck, aacid Cc: aacid, asturmlechner
D6331: Make sure that the tsfiles target is generated
apol marked an inline comment as done.
apol added inline comments.
INLINE COMMENTS
> ltoscano wrote in KF5I18NMacros.cmake:178
> Before this line, in the original version before the changes, there was this
> line:
>
> string(REPLACE "@" "_" pmapc_target ${pmapc_target})
>
> Could it be still relevant?
You are right, I must have copied a version that didn't have it yet. Fine by
me, included.
REPOSITORY
R249 KI18n
REVISION DETAIL
https://phabricator.kde.org/D6331
To: apol, #frameworks, ltoscano, lueck, aacid
Cc: aacid, asturmlechner
D6331: Make sure that the tsfiles target is generated
apol updated this revision to Diff 16202. apol added a comment. - Restore the old behavior of KI18N_INSTALL_TS_FILES - Address luigi's comment REPOSITORY R249 KI18n CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6331?vs=16136&id=16202 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6331 AFFECTED FILES cmake/KF5I18NMacros.cmake To: apol, #frameworks, ltoscano, lueck, aacid Cc: aacid, asturmlechner
D6512: Add support for proposed tags addition in OCS 1.7
leinir added a comment. In https://phabricator.kde.org/D6512#121834, @apol wrote: > +1 looks sensible to me. Sweet :) > What's the OCS state in this regard? When will 1.7 be a thing? OCS 1.7 will be a thing hopefully in the not too distant future... Incidentally, i need to regain write access to the fdo wiki, where said standard is hosted, as i lost that when they changed all the access methods in... some long time ago. REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D6512 To: leinir, #knewstuff, apol, whiting, #kde_store Cc: #kde_store, #frameworks, ZrenBot, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck, sebas
D6513: Add support for Attica tags support
leinir created this revision. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This is to add support for the new tags support in Attica found in https://phabricator.kde.org/D6512, which is based on the proposal in https://phabricator.kde.org/T6133 to add tags support in the next version of OCS. As it depends on https://phabricator.kde.org/D6512, this patch should consequently not be merged before that one is. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6513 AFFECTED FILES src/attica/atticaprovider.cpp src/core/entryinternal.cpp src/core/entryinternal.h To: leinir, #knewstuff, apol, #kde_store, whiting Cc: #frameworks, #knewstuff, ZrenBot
D6512: Add support for proposed tags addition in OCS 1.7
apol added a comment. +1 looks sensible to me. What's the OCS state in this regard? When will 1.7 be a thing? REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D6512 To: leinir, #knewstuff, apol, whiting, #kde_store Cc: #kde_store, #frameworks, ZrenBot, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck, sebas
D6512: Add support for proposed tags addition in OCS 1.7
leinir added a dependent revision: D6513: Add support for Attica tags support. REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D6512 To: leinir, #knewstuff, apol, whiting, #kde_store Cc: #kde_store, #frameworks, ZrenBot, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck, sebas
D6512: Add support for proposed tags addition in OCS 1.7
leinir created this revision. leinir added a project: KDE Store. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This patch is to add support for the proposed addition of tags to the OCS standard version 1.7, as seen in https://phabricator.kde.org/T6133 It is supposed to: - handle the addition of tags to content and download items - not fail if they are not available (just returning an empty list) REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D6512 AFFECTED FILES src/content.cpp src/content.h src/contentparser.cpp src/downloaddescription.cpp src/downloaddescription.h src/parser.cpp To: leinir, #knewstuff, apol, whiting, #kde_store Cc: #kde_store, #frameworks, ZrenBot, akiraohgaki, alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck, sebas
Re: KTitleWidget and the native Mac style
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.
D6493: Add KF5WindowSystem to link interface
davidedmundson added a comment. Edit: I was wrong. We do need your first change but not the second change to cmakelists.txt REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D6493 To: asturmlechner, #plasma Cc: davidedmundson, #frameworks
Re: KTitleWidget and the native Mac style
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.
