Re: Review Request 118849: AppletQuickItem: Do not remember the popup dialog size

2014-06-20 Thread Marco Martin

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


this could maybe fix 
https://bugs.kde.org/show_bug.cgi?id=336070

but i'm a bit on the fence.
To me kida seems that to be really correct, one would allow either manually 
resizing popups xor allowing applets to resize themselves, the two things seems 
kinda exclusive.

the only thing that would work in both cases is applets setting their minimum 
and maximum size, this that should work correctly now, regardless of saved size.
to me either way is fine, i wasn't an huge fan of manual popup resizing, but if 
this is removed could be considered that could be to never return, since would 
make applets to assume they are the sole in charge of the resize of the popup

- Marco Martin


On June 20, 2014, 2:35 p.m., Vishesh Handa wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118849/
 ---
 
 (Updated June 20, 2014, 2:35 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 The popup dialog can currently never be resized, so it doesn't make
 sense to record its height in a config file. Additionally, this also
 causes problems when applet writers change the size of their plasmoid.
 Since the old size is saved in the config file, it is shown with the old
 dimensions instead of the new ones.
 
 When we implement dialog resizing at that time we can propogate a signal 
 which says that the applet has been manually resized and save it in a config 
 file. Though even that could be buggy since the plasmoid could later be much 
 smaller.
 
 
 Diffs
 -
 
   src/plasmaquick/appletquickitem.cpp a6e91f7 
   src/plasmaquick/private/appletquickitem_p.h 8b8010a 
 
 Diff: https://git.reviewboard.kde.org/r/118849/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Vishesh Handa
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 118849: AppletQuickItem: Do not remember the popup dialog size

2014-06-20 Thread Aleix Pol Gonzalez


 On June 20, 2014, 2:56 p.m., Marco Martin wrote:
  this could maybe fix 
  https://bugs.kde.org/show_bug.cgi?id=336070
  
  but i'm a bit on the fence.
  To me kida seems that to be really correct, one would allow either 
  manually resizing popups xor allowing applets to resize themselves, the two 
  things seems kinda exclusive.
  
  the only thing that would work in both cases is applets setting their 
  minimum and maximum size, this that should work correctly now, regardless 
  of saved size.
  to me either way is fine, i wasn't an huge fan of manual popup resizing, 
  but if this is removed could be considered that could be to never return, 
  since would make applets to assume they are the sole in charge of the 
  resize of the popup

I think we should make those dialogs resizable again, but then we need to think 
it through, for example only trying to set the size on load and storing it only 
when the user changed the size.

That can be a good feature to get in 5.1, for the moment we can remove the code 
that resizes to a setting nobody meant to save.

+1 to the patch.


- Aleix


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


On June 20, 2014, 2:35 p.m., Vishesh Handa wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118849/
 ---
 
 (Updated June 20, 2014, 2:35 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 The popup dialog can currently never be resized, so it doesn't make
 sense to record its height in a config file. Additionally, this also
 causes problems when applet writers change the size of their plasmoid.
 Since the old size is saved in the config file, it is shown with the old
 dimensions instead of the new ones.
 
 When we implement dialog resizing at that time we can propogate a signal 
 which says that the applet has been manually resized and save it in a config 
 file. Though even that could be buggy since the plasmoid could later be much 
 smaller.
 
 
 Diffs
 -
 
   src/plasmaquick/appletquickitem.cpp a6e91f7 
   src/plasmaquick/private/appletquickitem_p.h 8b8010a 
 
 Diff: https://git.reviewboard.kde.org/r/118849/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Vishesh Handa
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 118849: AppletQuickItem: Do not remember the popup dialog size

2014-06-20 Thread Vishesh Handa


 On June 20, 2014, 2:56 p.m., Marco Martin wrote:
  this could maybe fix 
  https://bugs.kde.org/show_bug.cgi?id=336070
  
  but i'm a bit on the fence.
  To me kida seems that to be really correct, one would allow either 
  manually resizing popups xor allowing applets to resize themselves, the two 
  things seems kinda exclusive.
  
  the only thing that would work in both cases is applets setting their 
  minimum and maximum size, this that should work correctly now, regardless 
  of saved size.
  to me either way is fine, i wasn't an huge fan of manual popup resizing, 
  but if this is removed could be considered that could be to never return, 
  since would make applets to assume they are the sole in charge of the 
  resize of the popup
 
 Aleix Pol Gonzalez wrote:
 I think we should make those dialogs resizable again, but then we need to 
 think it through, for example only trying to set the size on load and storing 
 it only when the user changed the size.
 
 That can be a good feature to get in 5.1, for the moment we can remove 
 the code that resizes to a setting nobody meant to save.
 
 +1 to the patch.


So, ship it?

As Aleix says, we do need manual resizing as well. But I don't think we're 
going to get that into 5.0.


- Vishesh


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


On June 20, 2014, 2:35 p.m., Vishesh Handa wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118849/
 ---
 
 (Updated June 20, 2014, 2:35 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 The popup dialog can currently never be resized, so it doesn't make
 sense to record its height in a config file. Additionally, this also
 causes problems when applet writers change the size of their plasmoid.
 Since the old size is saved in the config file, it is shown with the old
 dimensions instead of the new ones.
 
 When we implement dialog resizing at that time we can propogate a signal 
 which says that the applet has been manually resized and save it in a config 
 file. Though even that could be buggy since the plasmoid could later be much 
 smaller.
 
 
 Diffs
 -
 
   src/plasmaquick/appletquickitem.cpp a6e91f7 
   src/plasmaquick/private/appletquickitem_p.h 8b8010a 
 
 Diff: https://git.reviewboard.kde.org/r/118849/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Vishesh Handa
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 118849: AppletQuickItem: Do not remember the popup dialog size

2014-06-20 Thread Marco Martin

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

Ship it!


well, let's remove it for now

- Marco Martin


On June 20, 2014, 2:35 p.m., Vishesh Handa wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118849/
 ---
 
 (Updated June 20, 2014, 2:35 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 The popup dialog can currently never be resized, so it doesn't make
 sense to record its height in a config file. Additionally, this also
 causes problems when applet writers change the size of their plasmoid.
 Since the old size is saved in the config file, it is shown with the old
 dimensions instead of the new ones.
 
 When we implement dialog resizing at that time we can propogate a signal 
 which says that the applet has been manually resized and save it in a config 
 file. Though even that could be buggy since the plasmoid could later be much 
 smaller.
 
 
 Diffs
 -
 
   src/plasmaquick/appletquickitem.cpp a6e91f7 
   src/plasmaquick/private/appletquickitem_p.h 8b8010a 
 
 Diff: https://git.reviewboard.kde.org/r/118849/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Vishesh Handa
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 118849: AppletQuickItem: Do not remember the popup dialog size

2014-06-20 Thread Vishesh Handa

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

(Updated June 20, 2014, 3:24 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-framework


Description
---

The popup dialog can currently never be resized, so it doesn't make
sense to record its height in a config file. Additionally, this also
causes problems when applet writers change the size of their plasmoid.
Since the old size is saved in the config file, it is shown with the old
dimensions instead of the new ones.

When we implement dialog resizing at that time we can propogate a signal which 
says that the applet has been manually resized and save it in a config file. 
Though even that could be buggy since the plasmoid could later be much smaller.


Diffs
-

  src/plasmaquick/appletquickitem.cpp a6e91f7 
  src/plasmaquick/private/appletquickitem_p.h 8b8010a 

Diff: https://git.reviewboard.kde.org/r/118849/diff/


Testing
---


Thanks,

Vishesh Handa

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 118849: AppletQuickItem: Do not remember the popup dialog size

2014-06-20 Thread Commit Hook

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


This review has been submitted with commit 
d3de99eb1e59c30a7e0fa53c81dcf0af5cb35d67 by Vishesh Handa to branch master.

- Commit Hook


On June 20, 2014, 2:35 p.m., Vishesh Handa wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118849/
 ---
 
 (Updated June 20, 2014, 2:35 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 The popup dialog can currently never be resized, so it doesn't make
 sense to record its height in a config file. Additionally, this also
 causes problems when applet writers change the size of their plasmoid.
 Since the old size is saved in the config file, it is shown with the old
 dimensions instead of the new ones.
 
 When we implement dialog resizing at that time we can propogate a signal 
 which says that the applet has been manually resized and save it in a config 
 file. Though even that could be buggy since the plasmoid could later be much 
 smaller.
 
 
 Diffs
 -
 
   src/plasmaquick/appletquickitem.cpp a6e91f7 
   src/plasmaquick/private/appletquickitem_p.h 8b8010a 
 
 Diff: https://git.reviewboard.kde.org/r/118849/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Vishesh Handa
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel