zulip-desktop icon indicating copy to clipboard operation
zulip-desktop copied to clipboard

Feature Extension Re: #1360: Option to set default overall zoom for all servers

Open coxtrent opened this issue 10 months ago • 2 comments

What's this PR do? Resolves issue #1360 by adding option to synchronize zoom factor across all organizations. Beforehand, the zoom was per organization, making for something of a clunky and sometimes confusing experience.

Any background context you want to provide?

  • Added a new button/setting to app.renderer.js.pages.preference.general-section.ts
  • Used ipc to send a message that synchronizes the zoom factors to 1 when the button is turned on, added "sync-zooms" to type MainMessage and type RendererMessage in typed-ipc.ts
  • Added getZoomFactor() and setZoomFactor() methods to WebView. in app.renderer.js.components.webview.ts (breaking the abstraction is bad)
  • Added private member function syncZooms() to ServerManagerView that actually sets the values of all tabs' zoom factors to its one argument (default = 1), then call this function from the WebviewListeners for zoom functions. It must be handled here and not from the BrowserWindow object or else it will change the size of the sidebar with the organizations and settings, etc. The function has no effect if the option is off. This handles the synchronous zoom functionality and optionality almost completely outside the old functions while not changing pre-existing code.

You have tested this PR on:

  • [ ] Windows
  • [ ] Linux/Ubuntu
  • [ X ] macOS

coxtrent avatar Apr 27 '24 01:04 coxtrent

Thanks for the work here! Please clean up your commit history and post again to request a review. See here for guidelines.

A link to the chat.zulip.org discussion from this PR's description would also be helpful.

alya avatar Apr 27 '24 06:04 alya

Sure! I noticed a couple minor kinks in my implementation so I've been taking a bit more time to work on it. Thanks for the pointers by the way

coxtrent avatar Apr 29 '24 03:04 coxtrent