Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

coxtrent
Copy link

@coxtrent coxtrent commented Apr 27, 2024

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 coxtrent changed the title Resolves issue #1360: [Feat] Option to set default overall zoom for all servers Feature Extension Re: #1360: Option to set default overall zoom for all servers Apr 27, 2024
@alya
Copy link
Collaborator

alya commented Apr 27, 2024

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.

@coxtrent
Copy link
Author

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

@zulipbot
Copy link
Member

Heads up @coxtrent, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants