Skip to content

KShortcutsDialog: code refactoring

Ahmad Samir requested to merge work/ahmad/shortcuts-dialog into master
  • Start deprecating the saveSettings logic, I think we always want to save the changes if the user clicks OK; also looking at KDE code, saveSettings was mostly always true; for the newly added method(s) just set saveSettings to true
  • Flip the modality default, i.e. it's now non-modal by default, users of the class can make it modal by calling setModal(true); the point being able to interact with the parent app while configuring the shortcuts; if you close the parent app while the shortcuts dialog is open, the dialog is simply closed; this might have caused issues when using exec(), but now we always use show()
  • Add a new static method, showDialog(), which always opens the dialog using show(), no need for exec() and nested-eventloops and the potential issues that usually come with that
  • Update the API docs, some slots don't exist any more
  • Change static configure() method, the dialog needs to be created on the heap, otherwise if it's created on the stack and then opened by show() it'll be shown for a fraction of a second, then control goes out of the calling site, and it gets deleted...

Also add a new KXMLGUIFactory::showConfigureShortcutsDialog() method, that takes no args:

  • Looking at existing KDE code, KXMLGUIFactory::configureShortcuts(bool, bool) is almost always called with the default args, both true
  • This method is easier to use in the idiomatic KStandardAction::keyBindings() call, because the latter ultimately connects QAction::triggred(bool) with showConfigureShortcutsDialog() (which is fine, since it has no args, but is a bit more problematic with configureShortcuts(bool, bool), given it can be solved by simply using a lambda, but then again no code in KDE is using those bool args)
Edited by Ahmad Samir

Merge request reports