Repository navigation
feat(cpn): reorganise and refactor Edit Settings (Profile/Application/Simulator/Update) dialog - #7623
Conversation
|
Working on tree styled radio picker with radios grouped by manufacturer to replace the long dropdown combo box |
|
Oh, thanks Neil... wasn't going to look at that until next week :) |
|
Looking very nice, and should be good on my other two laptops given the much smaller screen footprint! Have only tried on macOS so far, and will take a bit of getting used to everything being collapsed by default... (although I wonder if that is actually strictly necessary since we can collapse on demand now 🤔 ). It seems the simulator page is wider than all the others... is it worth getting the initial preferences dialog window size to be the same as that, so it doesn't resize as you change through the pages? Also, it forces the shrink even if I resized the size manually and re-open it... claude suggested this if that helps / is of interest? Suggested fix: measure every tab's size hint once (each is invisible at this point in the constructor, so no flicker) and size the dialog to the largest, and only do that when there's no saved geometry to restore. --- a/companion/src/prefsedit/prefs_edit.cpp
+++ b/companion/src/prefsedit/prefs_edit.cpp
@@ -41,7 +41,7 @@ PrefsEditDialog::PrefsEditDialog(QWidget * parent, UpdateFactories * factories
ui->setupUi(this);
setWindowIcon(CompanionIcon("apppreferences.png"));
setAttribute(Qt::WA_DeleteOnClose);
- restoreGeometry(g.prefsEditGeo());
+ bool hasSavedGeo = restoreGeometry(g.prefsEditGeo());
PrefsProfilePanel *prefsProfPanel = new PrefsProfilePanel(this, firmware, board, profile);
PrefsPanel *profPanel = addTab(prefsProfPanel, tr("Radio Profile"));
@@ -61,7 +61,9 @@ PrefsEditDialog::PrefsEditDialog(QWidget * parent, UpdateFactories * factories
connect(prefsProfPanel, &PrefsProfilePanel::sdPathChanged, prefsUpdatePanel, &PrefsUpdatePanel::onSDPathChanged);
ui->tabWidget->setCurrentIndex(0);
- shrink();
+
+ if (!hasSavedGeo)
+ shrink();
}
PrefsEditDialog::~PrefsEditDialog()
@@ -158,7 +160,20 @@ void PrefsEditDialog::setMainWinHasDirtyChild(bool value)
void PrefsEditDialog::shrink()
{
- adjustSize();
+ // adjustSize() only accounts for the currently visible tab, since
+ // QStackedWidget doesn't lay out hidden pages. Walk every tab once so
+ // each panel reports its real size hint, and size the dialog to fit
+ // the largest one so switching tabs later doesn't resize the window.
+ QSize maxHint;
+ const int current = ui->tabWidget->currentIndex();
+
+ for (int i = 0; i < ui->tabWidget->count(); i++) {
+ ui->tabWidget->setCurrentIndex(i);
+ ui->tabWidget->currentWidget()->adjustSize();
+ maxHint = maxHint.expandedTo(ui->tabWidget->currentWidget()->sizeHint());
+ }
+
+ ui->tabWidget->setCurrentIndex(current);
+ adjustSize();
+ resize(maxHint.expandedTo(size()));
} |
is due to a double-delete prefs_edit.cpp:42 adds setAttribute(Qt::WA_DeleteOnClose) and correspondingly, mainwindow.cpp:461 lost its dialog->deleteLater() - That's a double-delete, not a mystery — done() already schedules the deletion
Claude is waffling about profile.fwName("") is no longer cleared on radio change. Old apppreferencesdialog.cpp:222 did profile.fwName("") alongside profile.fwType(newFw->getId()). Nothing in prefsedit/ does. FlashFirmwareDialog's ctor initialises fwName(g.profile[g.id()].fwName()), so after switching radio type, Flash Firmware prefills the previous radio's binary path. New code emits resetFirmware(), then falls through to Firmware::setCurrentVariant(firmware); fwchange = true; and on to panel->save() for every panel — so Reset now saves everything and emits firmwareProfileChanged() for a change that was just undone. And the postChanged lambda at line 74-76 does profile.generalSettings(QByteArray()); profile.timeStamp(QString()); — which the old dialog never did here. That wipes the profile's radio-settings backup the moment you pick a different radio in the picker, and undoFirmwareChange() cannot restore it. The Reset option no longer fully resets. And I'm not awake enough to even attempt to process that 🤪 I'm getting random instances where the preferences panels seem to go blank on Windows when interacting with the collapsible headers), but I've not figured out a reliable repro for that yet, or tell if it is windows specific yet. Might have some issues here also
Some potential cleanup
|
Working through the other points... |
Yeah I caught that but since I'm lazy and have been working on this for some time thought 'well if you change the type then fool on you' but the inconsistency needs fixing. |
|
So I solved my dirty variable issue. Steps to recreate:
Result dirty not updated and no prompt to Save
Result dirty was updated and prompt to Save pops up. |
Fix defers clearing until save confirmed |
Oh, is that all you had to do... just not click anywhere... 🤣 🤭
Expecially if you do it that way... but hey... I would have also liked it to have been a big red threatening button ... that does nothing when you press it ... you were half-way there 🤣 |
Fixed |
The rework should fix this I think... |
Fixed |
These are ok in their context. @pfeerick this completes my review and fixes for this round of feedback |
e609030 to
3e97b3f
Compare
|
A quick walkthrough last night on Linux, macOS and Windows seemed ok. Some followup perhaps - it seem the firmware language doesn't default to anything - not even the currently selected language? And I do wonder now that there are scrollbars if we really need the groups to collapse by default. |
I'll look into the language bug. I could have an option for the startup state either collapsed or expanded. Then next question becomes can we save the last state for every group or select start state for each group, etc. I'd be happy with one start option collapsed or expanded otherwise as more groups are added the maintenance goes up. |
That is all I would want also... I prefer expanded here vs the radio as we have bigger screens, better inputs, etc, so can easily scroll around... so the present model means need to keep opening up the groups if I'm after an option, which also opens up the "now where was that option put" can of worms... so, when will there be a search box in the preferences screen (just kidding!) 🤪 )? |
|
So the language bug is dev specific one in that the conversion checks for settings version <= 3.0 and setting revision so your settings have not been converted and thus the old firmware settings have not been split out into the new fields and your settings language is blank. I'll bump the settings revision and tweak the conversion check plus populate if blank after conversion with the OS default in a PR. |
Changing the radio in the Radio Profile settings left Default Internal Module and External Module Size at the previous radio's values. The internal module list was also built for the active profile's radio rather than the one being edited. A new profile starts from the default variant (TX16S), so switching it to e.g. NV14 or EL18 kept Multi instead of AFHDS2A / AFHDS3. The pre-#7623 preferences dialog reset these on a radio change. When the radio changes, rebuild the internal module list for the new radio and set both fields to its defaults, or back to the saved profile values when changing back to the profile's radio (e.g. on Reset). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Changing the radio in the Radio Profile settings left Default Internal Module and External Module Size at the previous radio's values. The internal module list was also built for the active profile's radio rather than the one being edited. A new profile starts from the default variant (TX16S), so switching it to e.g. NV14 or EL18 kept Multi instead of AFHDS2A / AFHDS3. The pre-#7623 preferences dialog reset these on a radio change. When the radio changes, rebuild the internal module list for the new radio and set both fields to its defaults, or back to the saved profile values when changing back to the profile's radio (e.g. on Reset). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since #7623 the radio profile stores its build options in fwOptions, separately from fwType, but the current firmware variant was still looked up from fwType alone. Every capability that depends on a build option (nogvars, haptic, internalelrs/crsf/multi, afhds2a/3, flyskygimbals, danger, ...) was therefore always off. Look the variant up from fwType plus the selected options, and refresh it when only the options are changed in the profile settings, not just when the radio type changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since #7623 the radio profile stores its build options in fwOptions, separately from fwType, but the current firmware variant was still looked up from fwType alone. Every capability that depends on a build option (nogvars, haptic, internalelrs/crsf/multi, afhds2a/3, flyskygimbals, danger, ...) was therefore always off. Look the variant up from fwType plus the selected options, and refresh it when only the options are changed in the profile settings, not just when the radio type changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since #7623 the radio profile stores its build options in fwOptions, separately from fwType, but the current firmware variant was still looked up from fwType alone. Every capability that depends on a build option (nogvars, haptic, internalelrs/crsf/multi, afhds2a/3, flyskygimbals, danger, ...) was therefore always off. Look the variant up from fwType plus the selected options, and refresh it when only the options are changed in the profile settings, not just when the radio type changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What users will notice:
Under the covers:
TODO as separate PRs (as this is big enough):
Some sample side by side screenshots:



