Review Request 119698: Save radio button index in QGroupBox that are composed only by radio buttons

[Albert Astals Cid]

> On ago. 10, 2014, 9:01 p.m., Aleix Pol Gonzalez wrote:
> > src/kconfigdialogmanager.cpp, line 61
> > <https://git.reviewboard.kde.org/r/119698/diff/1/?file=303639#file303639line61>
> >
> >     It could be a QSet<QGroupBox*>, you're already doing the cast and checking it's not null anyway.

It could be, but in other places i use the set, i have a QWidget* at hand and not a QGroupBox*, which would mean having to add a few more dynamic_casts for nothing. So i'd prefer not to do it.


> On ago. 10, 2014, 9:01 p.m., Aleix Pol Gonzalez wrote:
> > src/kconfigdialogmanager.cpp, line 227
> > <https://git.reviewboard.kde.org/r/119698/diff/1/?file=303639#file303639line227>
> >
> >     You probably want to remove the widget if the widget is deleted, even though you're not accessing through them.

kconfigdialogmanager is a more "static" thing, i don't think people is adding and removing widgets from the configuration dialog on "runtime", so adding extra code to remove the group box from the set when the groupbox is deleted seems like extra work that won't really give us any benefit.


On ago. 10, 2014, 9:01 p.m., Albert Astals Cid wrote:
> > And maybe you can use the more generic type QAbstractButton, only maybe, I'm unsure, up to you.

I'll go QAbstractButton


- Albert


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


On ago. 10, 2014, 8:28 p.m., Albert Astals Cid wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119698/
> -----------------------------------------------------------
> 
> (Updated ago. 10, 2014, 8:28 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> -------
> 
> We are suggesting people to port from deprecated KButtonGroup to QGroupBox but the dialog manager does not behave the same. This patch fixes it by assuming that in a groupbox that is composed exclusively by radio buttons and whose config item is an int to save the index of the checked radio button instead of if the group box itself is checked.
> 
> 
> Diffs
> -----
> 
>   src/kconfigdialogmanager.cpp 94d3cd1 
> 
> Diff: https://git.reviewboard.kde.org/r/119698/diff/
> 
> 
> Testing
> -------
> 
> KGeography frameworks with KButtonGroup ported to QGroupBox works.
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140811/7f3d14f7/attachment.html>