Review Request 121625: Port Kexi Table View to Qt 4

Friedrich W. H. Kossebau kossebau at kde.org
Mon Dec 29 03:17:32 GMT 2014 

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121625/#review72566
-----------------------------------------------------------


Not yet give a detailed testing, noted one issue (see wiki) while giving it a shoot though.
Skimmed code at least a little, a few comments here.
Hopefully can give some more time the next days. Perhaps still good enough to merge already and fix remaining issues in 2.9 branch, given it basically seems to work.


kexi/CMakeLists.txt
<https://git.reviewboard.kde.org/r/121625/#comment50591>

    IF ENDIF Can be completely removed



kexi/plugins/forms/kexiformscrollview.cpp
<https://git.reviewboard.kde.org/r/121625/#comment50592>

    Comment "no sorting" no longer true, so better remove.



kexi/widget/tableview/KexiTableScrollArea_p.cpp
<https://git.reviewboard.kde.org/r/121625/#comment50628>

    Why setting to 0? Seems unneeded in deconstructor and unusual, could get a comment why it is done (or be removed).



kexi/widget/utils/kexirecordnavigator.h
<https://git.reviewboard.kde.org/r/121625/#comment50629>

    Hm, why QPixmap and not QIcon? Multiple places might want different scales of the icon, also at runtime, when the UI is adapted at runtime to a different display device perhaps.
    Also not sure if the navigator class should be owner of these icons/pismaps, perhaps some own namespace area would be better, so UIs not using objects of the navigator class can just ignore the class.



kexi/widget/utils/kexirecordnavigator.cpp
<https://git.reviewboard.kde.org/r/121625/#comment50630>

    `putAside...` sounds like an action, perhaps better `isAside...`



kexi/widget/utils/kexirecordnavigator.cpp
<https://git.reviewboard.kde.org/r/121625/#comment50631>

    For a nice API, instead of being of `bool` type, the var `put` ideally would be an enum and also named `position`.
    Method could be named `setPositionRelativeToHorizontalScrollBar`



libs/db/tableviewdata.cpp
<https://git.reviewboard.kde.org/r/121625/#comment50632>

    For simpler to read code better invert condition and remove empty first branch:
    ```
    // no such column?
    if (column < 0 || column >= d->columns.count()) {
        d->sortColumn = -1;
        d->realSortColumn = -1;
        return;
    }
    ```


- Friedrich W. H. Kossebau


On Dez. 21, 2014, 10:27 nachm., Jarosław Staniek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121625/
> -----------------------------------------------------------
> 
> (Updated Dez. 21, 2014, 10:27 nachm.)
> 
> 
> Review request for Calligra, Andrius da Costa Ribas, Friedrich W. H. Kossebau, Adam Pigg, Radosław Wicik, and Wojciech Kosowicz.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Finally! A port that took 6 months, now largely functional. Changes are so cross-dependent, it's hard to split to smaller patches that result in something stable.
> 
> Changes along the way:
> * Harmonize API related to sorting with the Qt API
> * Port and clean up current column highlighting
> * Make shortcuts work
> * Make arrow keys ignore modifiers
> * Ensure the current cell is visible before trying to remove current record
> * Use separate macro for enabling painting debug
> * Use row, then col as args in cellSelected() and paintCell() methods (per convention of Qt)
>   This also fixes on ensureCellVisible() call when the args where swapped by mistake.
> * If possible don't move pos of current selection after sorting
> * Rename record -> row in table view for more generic API
> * Clicking on vertical header selects row
> * Hovering on vertical header highlights row
> * Display current record pointer as icon
> * Show mouse highlight in the vertical header when needed
> * Support leave event
> * Fix sizes and buttons in record navigator
> * Make parameter names consistent
> * Fix column resizing, column sorting, painting on scroll
> * Fix initializing widths of table columns
> * Remove unused variable and excessive debug
> * Put horizontal scrollbar aside of the record navigator when possible (but don't cover the navigator)
> 
> New features:
> * Lighter style of rows
> * Display "key" icon in primary key columns
> 
> 
> Diffs
> -----
> 
>   kexi/widget/utils/CMakeLists.txt 6942aaa 
>   kexi/widget/utils/kexirecordmarker.h db024f2 
>   kexi/widget/utils/kexirecordmarker.cpp 5ed82e4 
>   kexi/widget/utils/kexirecordnavigator.h 7bb8b12 
>   kexi/widget/tableview/kexitableedit.cpp ff44b7c 
>   kexi/widget/tableview/kexitableview.h 86e5446 
>   kexi/widget/tableview/kexitableview.cpp 492b8a8 
>   kexi/widget/tableview/kexitableview_p.h 38fae7b 
>   kexi/widget/tableview/kexitableview_p.cpp 1f64f52 
>   kexi/widget/tableview/kexitableviewheader.h 1201f2b 
>   kexi/widget/tableview/kexitableviewheader.cpp fcf3ade 
>   kexi/plugins/tables/kexitabledesigner_dataview.cpp 34ae4ef 
>   kexi/plugins/tables/kexitabledesignerview.h cfec74b 
>   kexi/plugins/tables/kexitabledesignerview.cpp 7aca480 
>   kexi/plugins/tables/kexitabledesignerview_p.h c52bd0b 
>   kexi/plugins/tables/kexitabledesignerview_p.cpp 9b2dcbd 
>   kexi/plugins/tables/kexitablepart.cpp b8fd0d7 
>   kexi/tests/newapi/tableview_test.h 092c2ff 
>   kexi/widget/dataviewcommon/kexidataawareobjectiface.h fe8a2de 
>   kexi/widget/dataviewcommon/kexidataawareobjectiface.cpp 948ba5d 
>   kexi/widget/dataviewcommon/kexidataawarepropertyset.h a131b89 
>   kexi/widget/dataviewcommon/kexidataawarepropertyset.cpp 779b24e 
>   kexi/widget/dataviewcommon/kexidataawareview.h 6604bd2 
>   kexi/widget/dataviewcommon/kexidataawareview.cpp 5473ec7 
>   kexi/widget/tableview/CMakeLists.txt 24e1cf1 
>   kexi/widget/tableview/KexiDataTableScrollArea.h PRE-CREATION 
>   kexi/widget/tableview/KexiDataTableScrollArea.cpp PRE-CREATION 
>   kexi/widget/tableview/KexiDataTableView.h PRE-CREATION 
>   kexi/widget/tableview/KexiDataTableView.cpp PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollArea.h PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollArea.cpp PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollAreaHeader.h PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollAreaHeader.cpp PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollAreaHeaderModel.h PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollAreaHeaderModel.cpp PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollAreaWidget.h PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollAreaWidget.cpp PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollArea_p.h PRE-CREATION 
>   kexi/widget/tableview/KexiTableScrollArea_p.cpp PRE-CREATION 
>   kexi/widget/tableview/kexiblobtableedit.cpp bece46f 
>   kexi/widget/tableview/kexicomboboxbase.cpp d64439d 
>   kexi/widget/tableview/kexicomboboxpopup.h d08ece2 
>   kexi/widget/tableview/kexicomboboxpopup.cpp 4eba57b 
>   kexi/widget/tableview/kexicomboboxtableedit.cpp 2ee4a0c 
>   kexi/widget/tableview/kexidatatable.h b64cbb7 
>   kexi/widget/tableview/kexidatatable.cpp f75e736 
>   kexi/widget/tableview/kexidatatableview.h 53f2a61 
>   kexi/widget/tableview/kexidatatableview.cpp 62c8760 
>   kexi/widget/tableview/kexitableedit.h 6da77e3 
>   kexi/formeditor/connectiondialog.h 627499e 
>   kexi/formeditor/connectiondialog.cpp 469e78a 
>   kexi/pics/tableview_pen.png f91eacf63e604300ca563a48bd749cde5d33cb6c 
>   kexi/pics/tableview_pen.xpm 3d2bd3e 
>   kexi/pics/tableview_plus.png bc858050cf2bd3bb6d83f4b590817074e816658c 
>   kexi/pics/tableview_plus.xpm de7019b 
>   kexi/pics/tableview_pointer.png PRE-CREATION 
>   kexi/pics/tableview_pointer.xpm PRE-CREATION 
>   kexi/plugins/forms/CMakeLists.txt b9ce947 
>   kexi/plugins/forms/kexiformscrollview.h f4aa2e2 
>   kexi/plugins/forms/kexiformscrollview.cpp fbfa560 
>   kexi/plugins/forms/kexiformview.h 2932861 
>   kexi/plugins/queries/kexiquerydesignerguieditor.cpp 9538688 
>   kexi/plugins/queries/kexiqueryview.h b987d81 
>   kexi/plugins/queries/kexiqueryview.cpp 2206214 
>   kexi/plugins/reports/kexireportview.cpp 047f949 
>   libs/db/tableviewcolumn.cpp 0687b0b 
>   libs/db/tableviewdata.h 42523c4 
>   libs/db/tableviewdata.cpp e4660a0 
>   kexi/widget/utils/kexirecordnavigator.cpp dd47b2c 
>   libs/db/tableviewcolumn.h f2370bc 
>   kexi/plugins/tables/kexitabledesigner_dataview.h ea9aba0 
>   kexi/CMakeLists.txt 55eac5b 
> 
> Diff: https://git.reviewboard.kde.org/r/121625/diff/
> 
> 
> Testing
> -------
> 
> Tested on tables and query designer: functional
> 
> 
> Thanks,
> 
> Jarosław Staniek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20141229/1055fa28/attachment.htm>

More information about the calligra-devel mailing list