'Re: Review Request: Add KMetaDataWidget,'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Add KMetaDataWidget,
From:       Aurélien Gâteau <agateau () kde ! org>
Date:       2010-03-21 20:03:07
Message-ID: 20100321200307.19496.93364 () localhost
[Download RAW message or body]



> On 2010-03-21 16:29:31, Aurélien Gâteau wrote:
> > I didn't look at individual lines closely as others have already did so. I think \
> > there was a misunderstanding regarding my suggestion on the initial request (as \
> > far as I remember what I suggested...). I think i suggested it would be more \
> > useful to have a widget to display a set of properties, but what I meant was that \
> > these properties do not have to be properties of a file. One could imagine for \
> > example a calendar showing the properties of a meeting. Since KMetaDataWidget is \
> > tied to files, I agree KMetaDataModel can be made private, at least for now. I \
> > must confess I also agree with Christopher about the name being a bit confusing. \
> > Even if, as Peter says "Qt has no ownership on the MVC pattern", I think Qt users \
> > have come to expect that classes suffixed with "Model" inherits from \
> > QAbstractItemModel or a subclass of it, in the same way that classes suffixed \
> > with "Widget" are expected to inherit from QWidget. 
> > One question though: I don't see the connection between KMetaDataWidget and \
> > KMetaDataConfigurationDialog. When does one create such a dialog? How does one \
> > link it to KMetaDataWidget?
> 
> Peter Penz wrote:
> As KMetaDataModel is a private API now, it won't be a problem to rename it. I'm \
> open for any suggestions, I somehow would like the name KMetaDataProvider as \
> Christoph said... What do you think? 
> Regarding KMetaDataWidget and KMetaDataConfigurationDialog: You can take Dolphin as \
> example. If you right click on the Information Panel and select "Configure...", you \
> can configure which of the meta data for the currently shown file type should be \
> shown at all. IMO this should not be possible only within Dolphin, but also in \
> other applications that might use this widget. 
> 

KMetaDataProvider sounds good.

About the dialog: what I was wondering was how was the developer supposed to "link" \
the dialog and the widget. Looking at the code I realized they communicate via the \
"Nepomuk KMetaDataConfigDialog" configuration group. Should the group be hardcoded? I \
don't really know. Are there other places in kdelibs with hardcoded config groups \
like this?


- Aurélien


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


On 2010-03-20 20:57:44, Peter Penz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3277/
> -----------------------------------------------------------
> 
> (Updated 2010-03-20 20:57:44)
> 
> 
> Review request for kdelibs, Sebastian Trueg, David Faure, and Aurélien Gâteau.
> 
> 
> Summary
> -------
> 
> The patch adds KMetaDataWidget, KMetaDataModel and KMetaDataConfigurationDialog as \
> public classes to kdelibs/kfile. The KMetaDataWidget allows an application in an \
> easy way to show meta data of a file (or several files). The widget also allows to \
> change meta data like tags, comments and rating (see \
> http://enzosworld.gmxhome.de/temp/metadatawidget.png or attached screenshot). \
> KMetaDataConfigurationDialog allows to configure which meta tags should be \
> hidden/shown. The classes also work without Nepomuk (and show only very basic meta \
> data like size, permissions, ...). It is possible for applications to add custom \
> meta data if wanted (including widgets to manipulate this meta data). 
> The classes have been used by Dolphin internally until now and have originally been \
> written by Sebastian Trüg. After the request from Tom Albers and Oliver \
> Heidbüchel to integrate the widget also in Mailody/Okular I've adjusted the \
> classes to get them ready for a kdelibs-integration. I'd also like to to adjust \
> KPropertiesDialog later to use this widget. 
> I'd ask mainly to look at the files kfile/kmetadatawidget.h, kfile/kmetadatamodel.h \
> and kfile/kmetadataconfigurationdialog.h, the other APIs are internal. 
> Thanks!
> 
> 
> Diffs
> -----
> 
> trunk/KDE/kdelibs/CMakeLists.txt 1103259 
> trunk/KDE/kdelibs/config-nepomuk.h.cmake PRE-CREATION 
> trunk/KDE/kdelibs/kfile/CMakeLists.txt 1103259 
> trunk/KDE/kdelibs/kfile/kcommentwidget.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kcommentwidget_p.h PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kedittagsdialog.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kedittagsdialog_p.h PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kloadmetadatathread.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kloadmetadatathread_p.h PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kmetadataconfigurationdialog.h PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kmetadataconfigurationdialog.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kmetadatamodel.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kmetadatamodel_p.h PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kmetadatawidget.h PRE-CREATION 
> trunk/KDE/kdelibs/kfile/kmetadatawidget.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/knfotranslator.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/knfotranslator_p.h PRE-CREATION 
> trunk/KDE/kdelibs/kfile/ktaggingwidget.cpp PRE-CREATION 
> trunk/KDE/kdelibs/kfile/ktaggingwidget_p.h PRE-CREATION 
> trunk/KDE/kdelibs/nepomuk/core/ui/CMakeLists.txt 1103259 
> trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.h 1103259 
> trunk/KDE/kdelibs/nepomuk/core/ui/nepomukmassupdatejob.cpp 1103259 
> 
> Diff: http://reviewboard.kde.org/r/3277/diff
> 
> 
> Testing
> -------
> 
> Tested in Dolphin. An early version has been tested also in Mailody and Okular.
> 
> 
> Screenshots
> -----------
> 
> KMetaDataWidget
> http://reviewboard.kde.org/r/3277/s/330/
> 
> 
> Thanks,
> 
> Peter
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic