Review Request: Refactoring of Collection config UI code

Bart Cerneels bart.cerneels at kde.org
Mon Oct 8 12:05:46 UTC 2012


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

Ship it!


You might want to try the Ui::Class as member approach. But this code is already much better then what we had before. Less UI defined in C++ at least.

We should do this for all our dialogs and custom widgets, have a feeling it would solve quite a few layout glitches. One clear example is the likeback dialog.


src/configdialog/dialogs/CollectionConfig.h
<http://git.reviewboard.kde.org/r/106762/#comment15899>

    You could also use Ui_CollectionConfig *m_ui and do a m_ui->setupUi(this) in the constructor. The class would only derive from QWidget.
    It's a personal preference of mine so not obligated, but it does lead to better UI/logic separation in my experience.



src/configdialog/dialogs/CollectionConfig.ui
<http://git.reviewboard.kde.org/r/106762/#comment15896>

    This should be MinimumExpanding



src/configdialog/dialogs/CollectionConfig.ui
<http://git.reviewboard.kde.org/r/106762/#comment15897>

    This stretch should be the highest in the dialog (actually the only non 0) so this widget will be the only one to expand when resizing the dialog.



src/configdialog/dialogs/CollectionConfig.ui
<http://git.reviewboard.kde.org/r/106762/#comment15898>

    This one probably should not expand. As long as the combo is large enough to hold the longest string it's OK. Wide comboboxes are ugly.


- Bart Cerneels


On Oct. 8, 2012, 11:31 a.m., Mark Kretschmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106762/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2012, 11:31 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Refactoring the Collection config UI code, by moving move more stuff into the UI file. Also, a resizing issue of the UI was fixed, as well as some other small issues.
>     
> There is one remaining issue with this patch:
> The TreeView in the UI takes up too much vertical space, although the SizePolicy is set to Minimum. I can't figure out why it does that, so I'd welcome help.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 9efeb20 
>   src/configdialog/dialogs/CollectionConfig.h 02c80e1 
>   src/configdialog/dialogs/CollectionConfig.cpp 42ffa1f 
>   src/configdialog/dialogs/CollectionConfig.ui 692c3d9 
>   src/dialogs/CollectionSetup.h 23d717a 
>   src/dialogs/CollectionSetup.cpp 5ae0294 
>   src/widgets/CollectionSetupTreeView.h PRE-CREATION 
>   src/widgets/CollectionSetupTreeView.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106762/diff/
> 
> 
> Testing
> -------
> 
> Tested visual sanity of the UI, tested connections with config.
> 
> 
> Thanks,
> 
> Mark Kretschmann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20121008/a9a04c33/attachment.html>


More information about the Amarok-devel mailing list