[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
From:       "Kevin Ottens" <ervin () kde ! org>
Date:       2014-06-18 5:30:40
Message-ID: 20140618053040.3734.23309 () probe ! kde ! org
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


> On June 17, 2014, 4:11 p.m., Kevin Ottens wrote:
> > Marco, any chance for a second run of review on that patch?
> 
> Marco Martin wrote:
> i still not like this patch..
> 
> Kevin Ottens wrote:
> Should be dropped then? Or it can get somewhere?
> Feel free to drop it if you consider it a dead end. I prefer that to open patches \
> lingering on forever. 
> Marco Martin wrote:
> I don't seem to be able to do so.
> are permissions for doing so to be asked somewhere?

I thought you could. :-)
I can close it for you if you want, David E. should be able to as well. Should we \
discard it?


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118538/#review60277
-----------------------------------------------------------


On June 4, 2014, 10:43 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118538/
> -----------------------------------------------------------
> 
> (Updated June 4, 2014, 10:43 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Add a property containing the real edge a dialog is shown on.
> 
> If a dialog is meant to be shown on the left of an item, if there is not
> enough space it will be shown on the right.
> 
> We need to keep track of where we're actually shown in order to show the
> correct borders and perform the animation in the correct direction.
> 
> It is exposed as a property in case the dialog itself needs to know
> which side of the parent item it is, for example if needs to show an
> arrow to the parent.
> 
> --
> 
> Fixed some serious copy + paste fails in the hitting the bottom edge of
> the screen check.
> 
> 
> Diffs
> -----
> 
> src/plasmaquick/dialog.cpp 2f8227c 
> src/plasmaquick/dialog.h 4009222 
> 
> Diff: https://git.reviewboard.kde.org/r/118538/diff/
> 
> 
> Testing
> -------
> 
> Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations.
> (i.e placed near left edge, showed item with location set to both left and right)
> 
> 
> Thanks,
> 
> David Edmundson
> 
> 


[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/118538/">https://git.reviewboard.kde.org/r/118538/</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 17th, 2014, 4:11 p.m. UTC, <b>Kevin \
Ottens</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;">Marco, any chance for a second run of review on that patch?</pre>  \
</blockquote>




 <p>On June 17th, 2014, 4:17 p.m. UTC, <b>Marco Martin</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;">i still not like this \
patch..</pre>  </blockquote>





 <p>On June 17th, 2014, 4:35 p.m. UTC, <b>Kevin Ottens</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;">Should be dropped then? \
Or it can get somewhere? Feel free to drop it if you consider it a dead end. I prefer \
that to open patches lingering on forever.</pre>  </blockquote>





 <p>On June 17th, 2014, 4:41 p.m. UTC, <b>Marco Martin</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;">I don&#39;t seem to be \
able to do so. are permissions for doing so to be asked somewhere?</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;">I thought you could. :-) \
I can close it for you if you want, David E. should be able to as well. Should we \
discard it?</pre> <br />










<p>- Kevin</p>


<br />
<p>On June 4th, 2014, 10:43 p.m. UTC, David Edmundson wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for KDE Frameworks and Plasma.</div>
<div>By David Edmundson.</div>


<p style="color: grey;"><i>Updated June 4, 2014, 10:43 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-framework
</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;">Add a property containing the real edge a dialog is shown on.

If a dialog is meant to be shown on the left of an item, if there is not
enough space it will be shown on the right.

We need to keep track of where we&#39;re actually shown in order to show the
correct borders and perform the animation in the correct direction.

It is exposed as a property in case the dialog itself needs to know
which side of the parent item it is, for example if needs to show an
arrow to the parent.

--

Fixed some serious copy + paste fails in the hitting the bottom edge of
the screen check.


</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;">Ran dialog_positioning.qml in all 8 combinations of edges + preferred \
locations. (i.e placed near left edge, showed item with location set to both left and \
right) </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>src/plasmaquick/dialog.cpp <span style="color: grey">(2f8227c)</span></li>

 <li>src/plasmaquick/dialog.h <span style="color: grey">(4009222)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/118538/diff/" style="margin-left: \
3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>



_______________________________________________
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