Re: Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-26 Thread David Rosca 

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

(Updated Nov. 26, 2015, 9:05 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Martin Klapetek.


Changes
---

Submitted with commit ea03996f34d9d22083bdbe5fcdc11910e50253b7 by David Rosca 
to branch Plasma/5.5.


Bugs: 355798
https://bugs.kde.org/show_bug.cgi?id=355798


Repository: bluedevil


Description
---

Set the minimum window size to be twice the width of PIN label.


Diffs
-

  src/wizard/pages/pairing.cpp 6d1eac0 

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


Testing
---

The window gets resized now if there is not enough free space for PIN.


Thanks,

David Rosca

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

Re: Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-25 Thread David Rosca 

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

(Updated Nov. 25, 2015, 5:03 p.m.)


Review request for Plasma and Martin Klapetek.


Changes
---

setMinimumSize(sizeHint())


Bugs: 355798
https://bugs.kde.org/show_bug.cgi?id=355798


Repository: bluedevil


Description
---

Set the minimum window size to be twice the width of PIN label.


Diffs (updated)
-

  src/wizard/pages/pairing.cpp 6d1eac0 

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


Testing
---

The window gets resized now if there is not enough free space for PIN.


Thanks,

David Rosca

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

Re: Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-25 Thread David Rosca 


> On Nov. 25, 2015, 2:08 p.m., Sebastian Kügler wrote:
> > src/wizard/pages/pairing.cpp, line 111
> > 
> >
> > The *2 seems fairly random. Can't we set the right minimum width 
> > derived from the pinNumber? Given the pin numbers display is huge, wouldn't 
> > this lead to a very large widget? Also, distinction between minimumSize and 
> > preferredSize would be more useful here.

The issue is that the size constraints are not updated when the pin number is 
set (not sure why). So I've changed it to explicitly set wizard's minimumSize 
to the wizard's sizeHint.


- David


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


On Nov. 25, 2015, 5:03 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126160/
> ---
> 
> (Updated Nov. 25, 2015, 5:03 p.m.)
> 
> 
> Review request for Plasma and Martin Klapetek.
> 
> 
> Bugs: 355798
> https://bugs.kde.org/show_bug.cgi?id=355798
> 
> 
> Repository: bluedevil
> 
> 
> Description
> ---
> 
> Set the minimum window size to be twice the width of PIN label.
> 
> 
> Diffs
> -
> 
>   src/wizard/pages/pairing.cpp 6d1eac0 
> 
> Diff: https://git.reviewboard.kde.org/r/126160/diff/
> 
> 
> Testing
> ---
> 
> The window gets resized now if there is not enough free space for PIN.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-25 Thread David Rosca 


> On Nov. 25, 2015, 5:35 p.m., Martin Klapetek wrote:
> > As you don't implement your own sizeHint() in the wizard - can you be sure 
> > that QWizard::sizeHint() will always return sensible value?
> > 
> > As an alternative, you could just scale the font to always fit the wizard. 
> > That would also help with resizing the window (for whatever reason).

It does, it just ignores it if the sizeHint() was changed after 
initializePage(). Setting wizard sizepolicy to minimum has also no effect 
either.

That's why wizard->setMinimumSize(wizard->sizeHint()) in second diff works.


- David


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


On Nov. 25, 2015, 5:03 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126160/
> ---
> 
> (Updated Nov. 25, 2015, 5:03 p.m.)
> 
> 
> Review request for Plasma and Martin Klapetek.
> 
> 
> Bugs: 355798
> https://bugs.kde.org/show_bug.cgi?id=355798
> 
> 
> Repository: bluedevil
> 
> 
> Description
> ---
> 
> Set the minimum window size to be twice the width of PIN label.
> 
> 
> Diffs
> -
> 
>   src/wizard/pages/pairing.cpp 6d1eac0 
> 
> Diff: https://git.reviewboard.kde.org/r/126160/diff/
> 
> 
> Testing
> ---
> 
> The window gets resized now if there is not enough free space for PIN.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-25 Thread Martin Klapetek 

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

Ship it!


Ok, I'll trust you.

- Martin Klapetek


On Nov. 25, 2015, 6:03 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126160/
> ---
> 
> (Updated Nov. 25, 2015, 6:03 p.m.)
> 
> 
> Review request for Plasma and Martin Klapetek.
> 
> 
> Bugs: 355798
> https://bugs.kde.org/show_bug.cgi?id=355798
> 
> 
> Repository: bluedevil
> 
> 
> Description
> ---
> 
> Set the minimum window size to be twice the width of PIN label.
> 
> 
> Diffs
> -
> 
>   src/wizard/pages/pairing.cpp 6d1eac0 
> 
> Diff: https://git.reviewboard.kde.org/r/126160/diff/
> 
> 
> Testing
> ---
> 
> The window gets resized now if there is not enough free space for PIN.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-25 Thread Martin Klapetek 

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


As you don't implement your own sizeHint() in the wizard - can you be sure that 
QWizard::sizeHint() will always return sensible value?

As an alternative, you could just scale the font to always fit the wizard. That 
would also help with resizing the window (for whatever reason).

- Martin Klapetek


On Nov. 25, 2015, 6:03 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126160/
> ---
> 
> (Updated Nov. 25, 2015, 6:03 p.m.)
> 
> 
> Review request for Plasma and Martin Klapetek.
> 
> 
> Bugs: 355798
> https://bugs.kde.org/show_bug.cgi?id=355798
> 
> 
> Repository: bluedevil
> 
> 
> Description
> ---
> 
> Set the minimum window size to be twice the width of PIN label.
> 
> 
> Diffs
> -
> 
>   src/wizard/pages/pairing.cpp 6d1eac0 
> 
> Diff: https://git.reviewboard.kde.org/r/126160/diff/
> 
> 
> Testing
> ---
> 
> The window gets resized now if there is not enough free space for PIN.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-25 Thread Sebastian Kügler 

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



src/wizard/pages/pairing.cpp (line 111)


The *2 seems fairly random. Can't we set the right minimum width derived 
from the pinNumber? Given the pin numbers display is huge, wouldn't this lead 
to a very large widget? Also, distinction between minimumSize and preferredSize 
would be more useful here.



src/wizard/pages/pairing.cpp (line 143)


same as above concern.


- Sebastian Kügler


On Nov. 24, 2015, 9:36 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126160/
> ---
> 
> (Updated Nov. 24, 2015, 9:36 p.m.)
> 
> 
> Review request for Plasma and Martin Klapetek.
> 
> 
> Bugs: 355798
> https://bugs.kde.org/show_bug.cgi?id=355798
> 
> 
> Repository: bluedevil
> 
> 
> Description
> ---
> 
> Set the minimum window size to be twice the width of PIN label.
> 
> 
> Diffs
> -
> 
>   src/wizard/pages/pairing.cpp 6d1eac0 
> 
> Diff: https://git.reviewboard.kde.org/r/126160/diff/
> 
> 
> Testing
> ---
> 
> The window gets resized now if there is not enough free space for PIN.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Review Request 126160: Wizard: Set minimum size of window to ensure entire PIN is visible

2015-11-24 Thread David Rosca 

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

Review request for Plasma and Martin Klapetek.


Bugs: 355798
https://bugs.kde.org/show_bug.cgi?id=355798


Repository: bluedevil


Description
---

Set the minimum window size to be twice the width of PIN label.


Diffs
-

  src/wizard/pages/pairing.cpp 6d1eac0 

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


Testing
---

The window gets resized now if there is not enough free space for PIN.


Thanks,

David Rosca

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