search

elementary / terminal

Terminal emulator designed for elementary OS
https://elementary.io
GNU Lesser General Public License v3.0
406 stars 97 forks source link

Add `SettingsPopover` Class #699

Closed Marukesu closed 1 year ago

Marukesu commented 1 year ago

move the settings menu popover, and related functions to they own class.

the settings object is now, also used to keep the state between the widgets synchronized, and the actions provided by it make the new class mostly self-contained, only needing a single signal to indicate the window that the theme dialog should be shown.

jeremypw commented 1 year ago

@Marukesu This seems like a good idea especially as MainWindow.vala is larger than I would like. Could you fix the conflicts please?

Marukesu commented 1 year ago

@jeremypw, sorry for the delay in rebasing this, should be good to review.

don't know what caused the CI error after the rebase, but with the --print-errorlogs option, it should be more easy to know what test case failed in the future.

jeremypw commented 1 year ago

Despite the coding technicalities there do not seem to be any regressions in operation.

jeremypw commented 1 year ago

@Marukesu Another regression with this is that the font scale is not set individually on each tab.

Marukesu commented 1 year ago

I'm tempted to revert the ActionGroup change for now, the widget won't be as self-contained as i would like, but would still make this a good start in reducing the MainWindow scope.

jeremypw commented 1 year ago

Yes two smaller PRs would be easier to review.