Differential D21867

[Yakuake] add blur effect support
ClosedPublic
Actions

Authored by owenchia on Jun 17 2019, 2:00 AM.

Details

Reviewers
hein
Group Reviewers
Yakuake
Commits
R369:ed693e2a1c01: [Yakuake] add blur effect support
Summary

add an option to yakuakerc:

[Appearance]
Blur=true

see also:
https://www.reddit.com/r/kde/comments/6jzuwu/konsole_and_yakuake_blur/
https://www.reddit.com/r/kde/comments/c19yfj/simple_code_fix_yakuakes_blur_support/

Diff Detail

Repository
R369 Yakuake
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
owenchia requested review of this revision.Jun 17 2019, 2:00 AM
owenchia created this revision.
owenchia edited the summary of this revision. (Show Details)
ngraham added a subscriber: ngraham.EditedJun 17 2019, 4:52 AM

Thanks very much for the patch!

Is having this on the best default? This is not actually a sarcastic rhetorical question; I'm genuinely asking since I don't actually use Yakuake.

owenchia added a comment.Jun 17 2019, 6:38 AM
In D21867#481044, @ngraham wrote:

Thanks very much for the patch!

Is having this on the best default? This is not actually a sarcastic rhetorical question; I'm genuinely asking since I don't actually use Yakuake.

I'm not a expert, but i think it's the best to having this blur effect on default.
Also it's very easy to turn it off if necessary.

ngraham added a comment.Jun 17 2019, 1:43 PM
In D21867#481071, @owenchia wrote:
In D21867#481044, @ngraham wrote:

Thanks very much for the patch!

Is having this on the best default? This is not actually a sarcastic rhetorical question; I'm genuinely asking since I don't actually use Yakuake.

I'm not a expert, but i think it's the best to having this blur effect on default.

Maybe, but that's a design change, not just the addition of a new feature, so now the title/commit message is not accurate; you're not just adding support for blur, you're turning it on by default.

You need to either change the commit message to reflect the actual changes, or (preferably) only add support in this patch, and turn it on by default in another patch.

owenchia updated this revision to Diff 59999.Jun 17 2019, 1:59 PM

change Blur option's default value to false.

owenchia added a comment.Jun 17 2019, 2:14 PM
In D21867#481235, @ngraham wrote:

Maybe, but that's a design change, not just the addition of a new feature, so now the title/commit message is not accurate; you're not just adding support for blur, you're turning it on by default.

You need to either change the commit message to reflect the actual changes, or (preferably) only add support in this patch, and turn it on by default in another patch.

I changed it to false by default now.
Actually, although this option is turned on you still need to enable blur and transparency in Konsole profile configuration, this is the reason I decided to turn it on by default in the first place.

hein accepted this revision.Jul 2 2019, 1:45 AM

I've resisted patches like this for years now hoping someone would implement this properly in the Konsole KPart as part of the profile system where translucency is regulated, but no one did, so let's go with this.

This revision is now accepted and ready to land.Jul 2 2019, 1:45 AM
ngraham added a comment.Jul 4 2019, 6:19 PM

@owenchia, can you provide your email address so we can land this patch with correct authorship information? Thanks!

owenchia added a comment.Jul 8 2019, 4:47 PM
In D21867#491007, @ngraham wrote:

@owenchia, can you provide your email address so we can land this patch with correct authorship information? Thanks!

sure, my email address is aptx945@gmail.com, and my full name is Owen Chia.

Closed by commit R369:ed693e2a1c01: [Yakuake] add blur effect support (authored by owenchia, committed by ngraham). · Explain WhyJul 9 2019, 7:00 PM
This revision was automatically updated to reflect the committed changes.
ngraham added a comment.Jul 9 2019, 7:08 PM

Thanks Owen, very nice first patch! May it be just the first of many. :)

axionl added a subscriber: axionl.Nov 6 2019, 6:21 PM

Revision Contents
Changeset List

PathPackages
Mapp/config/yakuake.kcfg (5 lines)
Mapp/mainwindow.cpp (1 line)

Diff 61448

View Options

app/config/yakuake.kcfg

Loading...
View Options

app/mainwindow.cpp

Loading...