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