----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119824/#review65176 -----------------------------------------------------------
src/widgets/personnoteview.cpp <https://git.reviewboard.kde.org/r/119824/#comment45524> Given that you're deleting this on the first reload() call, let's remove creating it here, it just wastes resources, then make the ->deleteLater() call in the reload() have an if (d->mainWidget) {} around it src/widgets/plugins/note.h <https://git.reviewboard.kde.org/r/119824/#comment45526> The class is acutally a widget, so it should be named NoteWidget (also fix the header guard then) src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45528> These widgets should be created once in the constructor of this plugin, then only their content changed ...althought the m_person will have to stay I guess src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45527> remove src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45529> This should move to constructor as well src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45530> You shouldn't need "this->" here src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45531> --> constructor src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45532> remove "this->" src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45533> At least the error should be printed to console as qWarning() src/widgets/plugins/note.cpp <https://git.reviewboard.kde.org/r/119824/#comment45534> This is not enough, if I write some text, then delete it, the save button will still be enabled. Do m_saveBtn->setDisabled(m_noteEditor->toPlainText().isEmpty()); instead (and no this->) - Martin Klapetek On Aug. 18, 2014, 11:27 a.m., Nilesh Suthar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119824/ > ----------------------------------------------------------- > > (Updated Aug. 18, 2014, 11:27 a.m.) > > > Review request for Telepathy and Martin Klapetek. > > > Repository: libkpeople > > > Description > ------- > > Depends on https://git.reviewboard.kde.org/r/119735/ for KABC::Addressee > customs > > > Diffs > ----- > > src/widgets/CMakeLists.txt f637838 > src/widgets/personnoteview.h PRE-CREATION > src/widgets/personnoteview.cpp PRE-CREATION > src/widgets/plugins/note.h PRE-CREATION > src/widgets/plugins/note.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/119824/diff/ > > > Testing > ------- > > > Thanks, > > Nilesh Suthar > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
