fix: Klipper notifications visually broken since plasma 5.12
ClosedPublic

Authored by tillschafer on Mar 27 2018, 7:00 PM.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tillschafer created this revision.Mar 27 2018, 7:00 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 27 2018, 7:00 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
tillschafer requested review of this revision.Mar 27 2018, 7:00 PM

Wait for @fvogt and @kbroulik to confirm, but in general ok with me.
Please update the text in the .h file. and make it clear that these are in addition to the ones in the spec.

I was thinking if I should also add "th" for a more complete table tag set, but decided to keep it minimal.

update documentation

minor formatting

minor formatting

minor formatting

fvogt added a subscriber: broulik.Mar 27 2018, 7:24 PM

There was a discussion about this on IRC quite some time ago:

13:01 < d_ed> fvogt: kbroulik_: can I have your opinion on https://bugs.kde.org/show_bug.cgi?id=390375
13:01 < sKreamer> KDE bug 390375 in plasmashell (Notifications) "Klipper notifications visually broken since plasma 5.12" [normal,] https://bugs.kde.org/show_bug.cgi?id=390375
13:03 < kbroulik_> d_ed: lol "up", "current", "down"
13:03 < fvogt> Fixing klipper is IMO better
13:03 < kbroulik_> yes
13:03 < kbroulik_> but we also have no <font> tag anymore, right?
13:03 < d_ed> no, just "b"
13:04 < kbroulik_> so let's change it to
13:04 < kbroulik_> <b>current:</b> fobar

and later

16:28 < kbroulik_> d_ed: did you do a klipper patch for Bug 390375 else I will

So looks like this was just forgotten? Maybe @broulik remembers what the best option was.

IMO, aligned text makes it much more readable at a quick glance. Thus, just from the pure appearance POV I would prefer the current solution.

broulik accepted this revision.Mar 28 2018, 7:37 AM

I did a patch for Klipper to not use a table but it was quite awkward, I think allowing tables is fine

This revision is now accepted and ready to land.Mar 28 2018, 7:37 AM

Do you have commit access?

I do not have commit access. Could you please commit this for me? I think it should also go to 5.12 beside the master branch.

This revision was automatically updated to reflect the committed changes.