Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Edmundson]

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


does this close: https://bugs.kde.org/show_bug.cgi?id=354543

- David Edmundson


On Oct. 26, 2015, 10:43 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 26, 2015, 10:43 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Edmundson]

> On Oct. 29, 2015, 4:44 p.m., David Edmundson wrote:
> > does this close: https://bugs.kde.org/show_bug.cgi?id=354543
> 
> David Rosca wrote:
> Yes

could you close it then?
You don't want me claiming your bugzilla stats as my own :D


- David


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


On Oct. 26, 2015, 10:43 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 26, 2015, 10:43 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Rosca]

> On Oct. 29, 2015, 4:44 p.m., David Edmundson wrote:
> > does this close: https://bugs.kde.org/show_bug.cgi?id=354543

Yes


- David


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


On Oct. 26, 2015, 10:43 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 26, 2015, 10:43 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[Sebastian Kügler]

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



applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 


Why are you removing this? It seems unrelated to the rest of this patch.


- Sebastian Kügler


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[Marco Martin]

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

Ship it!


i like it, almost good to go


applets/systemtray/package/contents/ui/ExpandedRepresentation.qml (line 122)


this shouldn't be necessary, as hiddenView should still be just hidden when 
root.expandedTask != null, and when contentHeight < height shouldn't be 
interactive and scrollbar should be hidden


- Marco Martin


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Rosca]

> On Oct. 26, 2015, 10:13 a.m., Sebastian Kügler wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 81
> > 
> >
> > Why are you removing this? It seems unrelated to the rest of this patch.
> 
> David Rosca wrote:
> Because I couldn't find any use of this and thus I don't know if it 
> should be in ListView or in ScrollArea (that now has "hiddenView" id). I 
> guess it was used from C++ to get handle of the ListView, but isn't used 
> anymore?
> 
> Sebastian Kügler wrote:
> Alright, I think it can be removed, but please make sure by grepping for 
> "hiddenView" through the source. If it's not used, feel free to drop this 
> issue.

I actually did just that before removing it :) There is no use of it in 
plasma-workspace.


- David


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[Sebastian Kügler]

> On Oct. 26, 2015, 10:13 a.m., Sebastian Kügler wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 81
> > 
> >
> > Why are you removing this? It seems unrelated to the rest of this patch.
> 
> David Rosca wrote:
> Because I couldn't find any use of this and thus I don't know if it 
> should be in ListView or in ScrollArea (that now has "hiddenView" id). I 
> guess it was used from C++ to get handle of the ListView, but isn't used 
> anymore?

Alright, I think it can be removed, but please make sure by grepping for 
"hiddenView" through the source. If it's not used, feel free to drop this issue.


- Sebastian


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Rosca]

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

(Updated Oct. 26, 2015, 10:43 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit a2a9d41b334db36238232be8e093deeecab03514 by David Rosca 
to branch master.


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


Repository: plasma-workspace


Description
---

Show vertical scrollbar when hidden items doesn't fit in popup.


Diffs
-

  applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 

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


Testing
---

Works, although mouse scrolling works only on the scrollbar.


Thanks,

David Rosca

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Rosca]

> On Oct. 26, 2015, 10:13 a.m., Sebastian Kügler wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 81
> > 
> >
> > Why are you removing this? It seems unrelated to the rest of this patch.

Because I couldn't find any use of this and thus I don't know if it should be 
in ListView or in ScrollArea (that now has "hiddenView" id). I guess it was 
used from C++ to get handle of the ListView, but isn't used anymore?


- David


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[Marco Martin]

> On Oct. 26, 2015, 9:12 a.m., Marco Martin wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 129
> > 
> >
> > this shouldn't be necessary, as hiddenView should still be just hidden 
> > when root.expandedTask != null, and when contentHeight < height shouldn't 
> > be interactive and scrollbar should be hidden
> 
> David Rosca wrote:
> You mean the whole Qt.binding is not necessary, or just the 
> !root.expandedTask test? The !root.expandedTask is there to disable 
> interactivity when only the column with icons on the left is visible and the 
> plasmoid contents is on the right in popup (eg. when you open popup with 
> clicking on Show hidden icons arrow and then select some item).

The whole Component.onCompleted: handler can be removed, yes


- Marco


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Rosca]

> On Oct. 26, 2015, 9:12 a.m., Marco Martin wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 129
> > 
> >
> > this shouldn't be necessary, as hiddenView should still be just hidden 
> > when root.expandedTask != null, and when contentHeight < height shouldn't 
> > be interactive and scrollbar should be hidden

You mean the whole Qt.binding is not necessary, or just the !root.expandedTask 
test? The !root.expandedTask is there to disable interactivity when only the 
column with icons on the left is visible and the plasmoid contents is on the 
right in popup (eg. when you open popup with clicking on Show hidden icons 
arrow and then select some item).


- David


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Review Request 125775: System Tray: Add ScrollArea to hidden items view

[David Rosca]

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

Review request for Plasma.


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


Repository: plasma-workspace


Description
---

Show vertical scrollbar when hidden items doesn't fit in popup.


Diffs
-

  applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 

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


Testing
---

Works, although mouse scrolling works only on the scrollbar.


Thanks,

David Rosca

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