[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: Plasma Mediacenter: Move tabbar and browsinig
From: Alessandro Diaferia <alediaferia () gmail ! com>
Date: 2010-05-22 8:51:26
Message-ID: AANLkTimZY1EUXOAPUvphU88QKElXmMkUpkB8HEi9TIBP () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
2010/5/21 Christophe Olinger <olingerc@binarylooks.com>
>
>
> > On 2010-05-20 21:59:58, Alessandro Diaferia wrote:
> > > Just little annotations here and there. I like the way we're doing this
> > -) Don't worry about the bugs you mention, i'll likely investigate them as
> soon as the patch goes in.
>
> Shall I commit after applying your comments?
Just update the diff please. I'd like to do another round of review just in
case :)
>
> Thanks a lot for the bitwise OR explanation. Your 2 paragraphs explained it
> better than reading a whole wiki article about it! I sure wish I could
> follow one of your c++/qt courses. I am happy that you all convinced me to
> just get going instead of working alone on a useless project and trying to
> solve problems alone. Learning by doing TM. :-)
>
Glad about it :)
>
> The next patch will add a few animations here and there and use the new
> Plasma::Animations system. It will cover the medialyout class.
>
>
> - Christophe
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4050/#review5765
> -----------------------------------------------------------
>
>
> On 2010-05-19 10:50:29, Christophe Olinger wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://reviewboard.kde.org/r/4050/
> > -----------------------------------------------------------
> >
> > (Updated 2010-05-19 10:50:29)
> >
> >
> > Review request for Plasma and Alessandro Diaferia.
> >
> >
> > Summary
> > -------
> >
> > Review request for plasma-mediacenter
> > Now that the uberpatch is in, hacking has become easier again :-)
> > I decided to put the browsing control button (goPrevious) back into the
> browser, along with the tabbar. The tabbar can be used to change viewmodes
> later, e.g. by Artist, by Album, by Tag,...I have added an API that can be
> used to add pages to the tabbar. The states handle the adding of pages on
> entry. (Later we can add the connection between the tabbar and the
> modelpackage/dataengines of the browser).
> >
> > Question: do we need states for playing and browsing? (also for the
> floating mode?) At the moment there is one state for each, but with
> differences between playing and browsing
> >
> > Question: I am thinking of playing around with QML as soon as kubuntu has
> a qt4.7 package. Do you think I should? This would mean a lot of
> refactoring. I wanted to just use it for the two panels at the beginning.
> Maybe also the playlist.
> >
> > Current bugs (just so that I do not forget them and maybe one of you
> wants to look at them ;-)
> > Video Mode: When entering the videomode and clicking on play (with a
> video in the playlist) the videoplayer covers only half of the screen. When
> the video is stopped and again play is pressed, the videoplayer size is
> correct. Also, when I ply via the playlist (clicking on the item in the
> playlist) the videoplayer size is always correct?
> > Picture mode: I do not understand how to correctly position the widgets
> in the bottom bar. They are at the wrong position on state entry but as soon
> as I change a picture, they jump to the correct positions. Switching to the
> floating mode makes the widgets be a wrong positions also :-/
> >
> > We need a way to tell the browser to change the modelpackage. I guess via
> the plugin factory, but that is over my head ATM.
> >
> >
> > Diffs
> > -----
> >
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localpictures/localpictures.desktop
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localvideos/localvideos.desktop
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h
> 1128382
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp
> 1128382
> >
> > Diff: http://reviewboard.kde.org/r/4050/diff
> >
> >
> > Testing
> > -------
> >
> > All states were thoroughly tested and the above bugs were found (among
> others) There are lots of TODOs and FIXMEs in the code still. It's a WIP :-)
> >
> >
> > Thanks,
> >
> > Christophe
> >
> >
>
>
--
Alessandro Diaferia
KDE Developer
KDE e.V. member
[Attachment #5 (text/html)]
<br><br><div class="gmail_quote">2010/5/21 Christophe Olinger <span dir="ltr"><<a \
href="mailto:olingerc@binarylooks.com">olingerc@binarylooks.com</a>></span><br><blockquote \
class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, \
204, 204); padding-left: 1ex;"> <div class="im"><br>
<br>
> On 2010-05-20 21:59:58, Alessandro Diaferia wrote:<br>
> > Just little annotations here and there. I like the way we're doing this \
:-) Don't worry about the bugs you mention, i'll likely investigate them as \
soon as the patch goes in.<br> <br>
</div>Shall I commit after applying your comments? </blockquote><div>Just update the \
diff please. I'd like to do another round of review just in case :) \
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; \
border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
Thanks a lot for the bitwise OR explanation. Your 2 paragraphs explained it better \
than reading a whole wiki article about it! I sure wish I could follow one of your \
c++/qt courses. I am happy that you all convinced me to just get going instead of \
working alone on a useless project and trying to solve problems alone. Learning by \
doing TM. :-)<br> </blockquote><div>Glad about it :)<br> </div><blockquote \
class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, \
204, 204); padding-left: 1ex;"> <br>
The next patch will add a few animations here and there and use the new \
Plasma::Animations system. It will cover the medialyout class.<br> <font \
color="#888888"><br> <br>
- Christophe<br>
</font><div class="im"><br>
<br>
-----------------------------------------------------------<br>
This is an automatically generated e-mail. To reply, visit:<br>
<a href="http://reviewboard.kde.org/r/4050/#review5765" \
target="_blank">http://reviewboard.kde.org/r/4050/#review5765</a><br>
-----------------------------------------------------------<br>
<br>
<br>
</div><div><div></div><div class="h5">On 2010-05-19 10:50:29, Christophe Olinger \
wrote:<br> ><br>
> -----------------------------------------------------------<br>
> This is an automatically generated e-mail. To reply, visit:<br>
> <a href="http://reviewboard.kde.org/r/4050/" \
target="_blank">http://reviewboard.kde.org/r/4050/</a><br> > \
-----------------------------------------------------------<br> ><br>
> (Updated 2010-05-19 10:50:29)<br>
><br>
><br>
> Review request for Plasma and Alessandro Diaferia.<br>
><br>
><br>
> Summary<br>
> -------<br>
><br>
> Review request for plasma-mediacenter<br>
> Now that the uberpatch is in, hacking has become easier again :-)<br>
> I decided to put the browsing control button (goPrevious) back into the browser, \
along with the tabbar. The tabbar can be used to change viewmodes later, e.g. by \
Artist, by Album, by Tag,...I have added an API that can be used to add pages to the \
tabbar. The states handle the adding of pages on entry. (Later we can add the \
connection between the tabbar and the modelpackage/dataengines of the browser).<br>
><br>
> Question: do we need states for playing and browsing? (also for the floating \
mode?) At the moment there is one state for each, but with differences between \
playing and browsing<br> ><br>
> Question: I am thinking of playing around with QML as soon as kubuntu has a \
qt4.7 package. Do you think I should? This would mean a lot of refactoring. I wanted \
to just use it for the two panels at the beginning. Maybe also the playlist.<br>
><br>
> Current bugs (just so that I do not forget them and maybe one of you wants to \
look at them ;-)<br> > Video Mode: When entering the videomode and clicking on \
play (with a video in the playlist) the videoplayer covers only half of the screen. \
When the video is stopped and again play is pressed, the videoplayer size is correct. \
Also, when I ply via the playlist (clicking on the item in the playlist) the \
videoplayer size is always correct?<br>
> Picture mode: I do not understand how to correctly position the widgets in the \
bottom bar. They are at the wrong position on state entry but as soon as I change a \
picture, they jump to the correct positions. Switching to the floating mode makes the \
widgets be a wrong positions also :-/<br>
><br>
> We need a way to tell the browser to change the modelpackage. I guess via the \
plugin factory, but that is over my head ATM.<br> ><br>
><br>
> Diffs<br>
> -----<br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localpictures/localpictures.desktop \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localvideos/localvideos.desktop \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h \
1128382<br> > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp \
1128382<br> ><br>
> Diff: <a href="http://reviewboard.kde.org/r/4050/diff" \
target="_blank">http://reviewboard.kde.org/r/4050/diff</a><br> ><br>
><br>
> Testing<br>
> -------<br>
><br>
> All states were thoroughly tested and the above bugs were found (among others) \
There are lots of TODOs and FIXMEs in the code still. It's a WIP :-)<br> ><br>
><br>
> Thanks,<br>
><br>
> Christophe<br>
><br>
><br>
<br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Alessandro \
Diaferia<br>KDE Developer<br>KDE e.V. member<br><br>
_______________________________________________
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