> 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.
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? - Alexey ----------------------------------------------------------- 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
