| rkflx | |
| ngraham |
| Spectacle |
Per Nate Graham: "This UI control is a relic from a prior version of spectacle when the "Save" split button's menu was full of items. Now, it only has two, and this setting is checked by default. The use case for turning it off is pretty difficult to envision now. Therefore, in the interest of de-cluttering, let's remove it."
BUG: 390809
Before:
After:
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
Thanks for the patch (again!) ;)
As you've picked a junior job item, the actual code change is not that difficult, but let's first work on the commit message a bit. Please read https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch, and adapt your summary accordingly. https://bugs.kde.org/show_bug.cgi?id=390793#c1 and https://bugs.kde.org/show_bug.cgi?id=390809#c0 contain a bit of a rationale which would be good to have in the commit message too.
Let us know if you have any questions…
Also, try to look at Spectacle's help and see if you'd need to adapt doc/index.docbook too.
Migrating my comment from D10718, since I saw that one before this one. We'll use your patch, since you got in first. :)
Thanks! Great second patch. There's even more code clean-up we can do here, though. For example:
$ grep -ri UseDynamicSaveButton . ./src/SpectacleConfig.cpp:bool SpectacleConfig::useDynamicSaveButton() const ./src/SpectacleConfig.cpp:void SpectacleConfig::setUseDynamicSaveButton(bool enabled) ./src/SpectacleConfig.h: bool useDynamicSaveButton() const; ./src/SpectacleConfig.h: void setUseDynamicSaveButton(bool enabled); Binary file ./src/spectacle matches ./src/Gui/KSMainWindow.cpp: return cfgManager->useDynamicSaveButton() ? cfgManager->lastUsedSaveMode() : SaveMode::SaveAs;
You can also get rid of QCheckBox *mUseLastSaveAction; on generalOptionsPage.h
In the end, KSMainWindow::saveButtonMode() will be simplified, and always return the last used save mode.
Also, make sure you test the upgrade test case:
Also, please mention your testing process in the Test Plan section, and be sure to add the Spectacle group in the future.
Much better! Still needs a few things:
You are making progress ;)
| doc/index.docbook | ||
|---|---|---|
| 116 | Remove mention here too, please. | |
| src/Gui/KSMainWindow.cpp | ||
| 113–114 | Could be simplified even more: SpectacleConfig::instance()->lastUsedSaveMode() At this point the whole function is kinda pointless, so let's just remove it and add a direct call to the two switches. | |