> On June 28, 2015, 8:40 a.m., Dennis Nienhüser wrote: > > src/plugins/render/earthquake/EarthquakePlugin.cpp, line 167 > > <https://git.reviewboard.kde.org/r/124188/diff/1/?file=381446#file381446line167> > > > > can we get rid of the if clause and just call > > `m_ui->m_timeRangeNPastDaysRadioButton->setChecked( > > m_timeRangeNPastDays );` ? > > Alexey Bugrov wrote: > In this case, m_timeRangeNPastDaysRadioButton is then always checked > initially, irrespective of the m_timeRangeNPastDays in config. Anyway, is it > all that important? > > Dennis Nienhüser wrote: > It will start working when you put the radio buttons into the same > buttongroup, and remove the manual setEnabled signal connection. > > Alexey Bugrov wrote: > But they already are in the same buttongroup, mutually exclusive... The > code suggested would just lead to them both being unchecked if > m_timeRangeNPastDays is false. And signal connection is for the widgets to > the right of each button to be grayed out when the given button is unchecked. > I think it's better to gray them out, conceptually and aesthetically, no?
Ok, I'm probably misinterpreting the behavior of radio buttons when disabling them programmatically. Ignore my comment then. You're right about gray-out behavior of course. - Dennis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124188/#review81805 ----------------------------------------------------------- On June 27, 2015, 10:18 p.m., Alexey Bugrov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124188/ > ----------------------------------------------------------- > > (Updated June 27, 2015, 10:18 p.m.) > > > Review request for Marble and Torsten Rahn. > > > Repository: marble > > > Description > ------- > > A patch for Bug 300184 - Eartquake Plugin: Need to use config dialog to see > recent earth quakes > > > Diffs > ----- > > src/plugins/render/earthquake/EarthquakeConfigWidget.ui 0cc1804 > src/plugins/render/earthquake/EarthquakePlugin.h 1cb88b7 > src/plugins/render/earthquake/EarthquakePlugin.cpp 47f041c > > Diff: https://git.reviewboard.kde.org/r/124188/diff/ > > > Testing > ------- > > Done some testing, seems to work ok. Passes the unit tests too. > > > Thanks, > > Alexey Bugrov > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
