[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 124149: Rework the notifications sizing code
From: "Alexander Potashev" <aspotashev () gmail ! com>
Date: 2015-06-26 11:48:04
Message-ID: 20150626114804.20764.940 () mimi ! kde ! org
[Download RAW message or body]
--===============7670736083615747779==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
> On June 24, 2015, 12:32 p.m., Alexander Potashev wrote:
> > Sorry for off-topic, is it possible to see the full text that was elided? If not, I'm going to file a \
> > feature request on bko.
>
> Martin Klapetek wrote:
> No; notifications should really be short and meaningfull, placing 10 lines text inside a notification \
> kinda defeats the purpose of the notifications and such notifications should be fixed at the source.
I'm not asking to make the whole notification text visible right away. We can add a button that would \
expand the notification or open its full version in a dialog.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124149/#review81722
-----------------------------------------------------------
On June 22, 2015, 4:53 p.m., Martin Klapetek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124149/
> -----------------------------------------------------------
>
> (Updated June 22, 2015, 4:53 p.m.)
>
>
> Review request for Plasma and Kai Uwe Broulik.
>
>
> Bugs: 339588 and 349142
> https://bugs.kde.org/show_bug.cgi?id=339588
> https://bugs.kde.org/show_bug.cgi?id=349142
>
>
> Repository: plasma-workspace
>
>
> Description
> -------
>
> This reworks the notification sizing computation to use Layouts. This should fix all the popup sizing \
> issues there are (including a binding loop error).
> Now it also properly elides after max 4 lines of text.
>
>
> Diffs
> -----
>
> applets/notifications/package/contents/ui/NotificationItem.qml 176ae91
> applets/notifications/package/contents/ui/NotificationPopup.qml e4e5fad
>
> Diff: https://git.reviewboard.kde.org/r/124149/diff/
>
>
> Testing
> -------
>
> I've been running it for couple hours, all notifications have correct sizes, see screenshot
>
>
> File Attachments
> ----------------
>
> Screenshot
> https://git.reviewboard.kde.org/media/uploaded/files/2015/06/22/f4f7689c-2a95-46aa-8e51-e90d25d99bb2__notifications-layout.png
>
>
> Thanks,
>
> Martin Klapetek
>
>
--===============7670736083615747779==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124149/">https://git.reviewboard.kde.org/r/124149/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 24th, 2015, 12:32 p.m. UTC, <b>Alexander Potashev</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Sorry for off-topic, is it possible to see the full text that was elided? \
If not, I'm going to file a feature request on bko.</p></pre> </blockquote>
<p>On June 24th, 2015, 12:42 p.m. UTC, <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">No; notifications should really be short and meaningfull, placing 10 lines \
text inside a notification kinda defeats the purpose of the notifications and such notifications should \
be fixed at the source.</p></pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I'm not asking to make the whole notification text visible right away. We \
can add a button that would expand the notification or open its full version in a dialog.</p></pre> <br \
/>
<p>- Alexander</p>
<br />
<p>On June 22nd, 2015, 4:53 p.m. UTC, Martin Klapetek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma and Kai Uwe Broulik.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated June 22, 2015, 4:53 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=339588">339588</a>,
<a href="https://bugs.kde.org/show_bug.cgi?id=349142">349142</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid \
#b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">This reworks the notification sizing \
computation to use Layouts. This should fix all the popup sizing issues there are (including a binding \
loop error).</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Now it also properly elides after max 4 lines of text.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I've been running it for couple hours, all \
notifications have correct sizes, see screenshot</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applets/notifications/package/contents/ui/NotificationItem.qml <span style="color: \
grey">(176ae91)</span></li>
<li>applets/notifications/package/contents/ui/NotificationPopup.qml <span style="color: \
grey">(e4e5fad)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124149/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/06/22/f4f7689c-2a95-46aa-8e51-e90d25d99bb2__notifications-layout.png">Screenshot</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============7670736083615747779==--
[Attachment #3 (text/plain)]
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic