Re: [kopete-devel] Review Request: Add the ability of take pictures from a webcam in the the AvatarManager and set it as a new avatar

[afiestas]

> On None, Matt Rogers wrote:
> > Could we call the dialog class AvatarWebcamDialog instead?

Dialog renamed.


> On None, Matt Rogers wrote:
> > /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.cpp, lines 211-224
> > <http://reviewboard.kde.org/r/1573/diff/2/?file=11260#file11260line211>
> >
> >     you use d->mainWidget.listUserAvatar in multiple places. You might want to make a variable out of that.

I haven't change this because all the file is accessing listUserAvatar in the same way, so I'd like to keep the coding style. If you want we can change all the accesses adding it to the private class for example.

Thanks for all the review!


> On None, Matt Rogers wrote:
> > /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.cpp, line 256
> > <http://reviewboard.kde.org/r/1573/diff/2/?file=11260#file11260line256>
> >
> >     Yeah, there's no point in modifying something that isn't already modified by this patch. The whitespace could be cleaned up in a separate patch.

Removed in a posterior commit.

> On None, Matt Rogers wrote:
> > /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.cpp, lines 118-121
> > <http://reviewboard.kde.org/r/1573/diff/2/?file=11260#file11260line118>
> >
> >     it's cleaner, IMHO, to do the following:
> >    
> >     using Kopete::AV;
> >     VideoDevicePool* devicePool = VideoDevicePool::self();
> >     devicePool->scanDevices();
> >     devicePool->size();
> >    
> >     etc.
> >     Seems more readable to me.

Done!


> On None, Matt Rogers wrote:
> > /trunk/KDE/kdenetwork/kopete/libkopete/CMakeLists.txt, lines 149-153
> > <http://reviewboard.kde.org/r/1573/diff/2/?file=11256#file11256line149>
> >
> >     X11_LIBRARIES expands to nothing on Windows, so there's no need for the conditional. I think they disable the whole avdevice directory on windows anyways.

Conditional removed, thanks


- Alex


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1573/#review2380
-----------------------------------------------------------


On 2009-09-12 01:50:35, Alex Fiestas wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1573/
> -----------------------------------------------------------
>
> (Updated 2009-09-12 01:50:35)
>
>
> Review request for Kopete.
>
>
> Summary
> -------
>
> If the user has a webcam, a button will appear in his AvatarManager, once it is clicked the avatarfromwebcamdialog will appear, showing the frames captured from the device. Then the user will have to click on OK button and crop the image.
>
> A few points:
> 1-I've created a new internal method called cropAndSaveAvatar which shows the crop dialog and then, save the avatar. This method is shared between "from webcam" and "add picture" methods.
> 2-I wonder if the dialog should be renamed to "webcamdialog" or something like this, so we can open it a bit more to be used by everybody.
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdenetwork/kopete/libkopete/CMakeLists.txt 1022342
>   /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarfromwebcamdialog.h PRE-CREATION
>   /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarfromwebcamdialog.cpp PRE-CREATION
>   /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.h 1022342
>   /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.cpp 1022342
>   /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.ui 1022342
>
> Diff: http://reviewboard.kde.org/r/1573/diff
>
>
> Testing
> -------
>
> I've tested almost all possible cases, except build it in windows. I've tried to be windows aware, so the new videodevice usage won't break anything, but I can't test it!.
>
>
> Thanks,
>
> Alex
>
>

_______________________________________________
kopete-devel mailing list
kopete-devel@...
https://mail.kde.org/mailman/listinfo/kopete-devel