----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105240/#review15049 -----------------------------------------------------------
I tested your branch today. First of all thanks for the nice work. There are some issues that I found while testing your branch. *) In the Bibliography Database dialog, if after pressing Crtl+N and then I cancel the dialog Words crashes. *) If I open the same table thrice then in the Table selection drop down it is shown thrice *) On changing the database table either the search box text should be cleared/the results should be filtered for the new table. *) I feel that the option to change the search input types (RegEx/Fixed String/WildCards) should not be hidden inside the menu but rather shown below the search input box. *) There is no option to remove filter conditions *) Keeping the end user in mind the criteria for the filter condition can be a textual description rather that SQL operators (eg. "Not equal to" instead of "<>") *) A behaviour similar to my 3rd point above for filters. Either the filters should be cleared on table change or the filters should be applied to the new table. *) Data loss - While creating a new database table if the new database name is same as an existing .sqlite file then there should be a confirmation message instead of silently overwriting the original file *) Do not open *.sqllite files that do not contain the bibref table *) I get a lot of "QSqlDatabasePrivate::addDatabase: duplicate connection name 'qt_sql_default_connection', old connection removed." debug messages in the console please do remove it. libs/kotext/KoInlineCite.cpp <http://git.reviewboard.kde.org/r/105240/#comment11833> I did not get what are you refering to when you say that KoXmlWriter::addAttribute doesn't accept attribute name in QString? My comment was on using loop insted of all these if conditions plugins/textshape/dialogs/BibliographyDb.cpp <http://git.reviewboard.kde.org/r/105240/#comment11834> hmm...I'm not sure you got my comment here. All the columns I see here are of type TEXT which should make my comment valid. Feel free to ping me on IRC if you want clarification on any of my commments libs/kotext/BibliographyGenerator.cpp <http://git.reviewboard.kde.org/r/105240/#comment11832> Please start the else in the same line as the close brace } else if() { plugins/textshape/dialogs/EditFiltersDialog.cpp <http://git.reviewboard.kde.org/r/105240/#comment11835> Instead of hard coding value 62 use insertWidget. If the pre-operator is not there there will be no widget in the first column. if (hasPreCondition) { m_layout->insertWidget(0, m_preCond); } m_layout->insertWidget(1, m_field); ...so on - Gopalakrishna Bhat On June 16, 2012, 4:28 p.m., Smit Patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105240/ > ----------------------------------------------------------- > > (Updated June 16, 2012, 4:28 p.m.) > > > Review request for Calligra. > > > Description > ------- > > Bibliography database UI. > 1) create/load/save/edit table saved in SQLite file > 2) Search in database > 3) Apply custom filters to database > 4) Integrated this UI with existing insert citation for edit/create cite in > database > > > Diffs > ----- > > CMakeLists.txt 33cdabb > libs/kotext/BibliographyGenerator.cpp 15808cb > libs/kotext/KoInlineCite.h df4a2e9 > libs/kotext/KoInlineCite.cpp 6d949da > libs/kotext/KoInlineTextObjectManager.cpp e82664d > libs/kotext/KoTextEditor.cpp b3e16ba > plugins/chartshape/TODO 94efb62 > plugins/textshape/CMakeLists.txt 6253323 > plugins/textshape/ReferencesTool.h 491b933 > plugins/textshape/ReferencesTool.cpp 564b658 > plugins/textshape/dialogs/BibliographyDatabaseWindow.h PRE-CREATION > plugins/textshape/dialogs/BibliographyDatabaseWindow.cpp PRE-CREATION > plugins/textshape/dialogs/BibliographyDatabaseWindow.ui PRE-CREATION > plugins/textshape/dialogs/BibliographyDb.h PRE-CREATION > plugins/textshape/dialogs/BibliographyDb.cpp PRE-CREATION > plugins/textshape/dialogs/CitationInsertionDialog.h f5eac0b6 > plugins/textshape/dialogs/CitationInsertionDialog.cpp 14f9429 > plugins/textshape/dialogs/CitationInsertionDialog.ui 8f10f64 > plugins/textshape/dialogs/EditFiltersDialog.h PRE-CREATION > plugins/textshape/dialogs/EditFiltersDialog.cpp PRE-CREATION > plugins/textshape/dialogs/InsertCitationDialog.h PRE-CREATION > plugins/textshape/dialogs/InsertCitationDialog.cpp PRE-CREATION > plugins/textshape/dialogs/InsertCitationDialog.ui PRE-CREATION > plugins/textshape/dialogs/SimpleCitationBibliographyWidget.cpp 2d1c1de > plugins/textshape/dialogs/SimpleCitationBibliographyWidget.ui 55699bb > > Diff: http://git.reviewboard.kde.org/r/105240/diff/ > > > Testing > ------- > > > Thanks, > > Smit Patel > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel