search

nylas-mail-lives / nylas-mail

:love_letter: An extensible desktop mail app built on the modern web.
MIT License
474 stars 63 forks source link

Enable Zoom Options in Preferences #166

Open nirmit opened 6 years ago

nirmit commented 6 years ago

This attempts to fix #59 by adding the option in preferences to change the zoom factor. The current available values are 80%, 100% (default), 125%, 150%, 175% and 200%.

Fixes #59

dweremeichik commented 6 years ago

Oh, cool! How well does it work?

nirmit commented 6 years ago

It works pretty well.

125%

125

150%

150

200%

200
lamixer commented 6 years ago

Fantastic! I've been waiting for this.. Thank you! Please compile a new version with this pretty soon :-)

On Mon, 13 Nov, 2017 at 3:38 AM, Nirmit notifications@github.com wrote:

It works pretty well.

125%

150%

200%

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

dweremeichik commented 6 years ago

@nirmit is this one ready to merge, or do you still have some changes to make?

nirmit commented 6 years ago

Ready to merge! Can someone test this on Linux/Windows? I don't have either. :(

dweremeichik commented 6 years ago

I'm :+1: on this, I will give it a spin on Linux when I get a chance.

mikeseese commented 6 years ago

im :+1: if @dweremeichik is

dweremeichik commented 6 years ago

I'll merge after testing.

dweremeichik commented 6 years ago

This breaks for me after opening up an email. (It resets to default).

lamixer commented 6 years ago

That is what happens if you set the zoom level in the config file also -- you see the zoom but clicking on something makes it revert to normal.

On Tue, 21 Nov, 2017 at 12:18 AM, Dylan Weremeichik notifications@github.com wrote:

This breaks for me after opening up an email. (It resets to default).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

nirmit commented 6 years ago

Hmm... Once you set the zoom, can you check if the interfaceZoom value propagates to all the config files in ~/.nylas-mail. It seems that the old value lingers in one of the config files.

mikeseese commented 6 years ago

@nirmit do you have a status on this since the last comments? are you waiting on someone to test and provide feedback?

nirmit commented 6 years ago

@seesemichaelj the issue is that not all versions of the config.json get the updated interfaceZoom values when the user changes the default. Can you help here in making this consistent across the various config.jsons being created?

If I change the zoom options, don't click on an email from the thread list, quit Nylas, and reopen, the interfaceZoom is updated and used. The client then displays everything with the desired zoom.

mikeseese commented 6 years ago

Well no where in your code does it look like the config is ever being changed? Unfortunately I don't really know how to add a parameter to be written to the config file

nirmit commented 6 years ago

A change in settings emits a change event that triggers an atomic write. Am I missing something?

In this case, the write happens in one file and not the other 9. If you change the zoom settings, you can see the changed value in one of your config.

On Wed, Dec 13, 2017 at 9:31 PM Mike Seese notifications@github.com wrote:

Well no where in your code does it look like the config is ever being changed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nylas-mail-lives/nylas-mail/pull/166#issuecomment-351590112, or mute the thread https://github.com/notifications/unsubscribe-auth/AAaUJPxfpXoTB-4_EyfYVy3yxwJlIlmiks5tAIiegaJpZM4QaxzW .

mikeseese commented 6 years ago

The config.json files are timestamped backups. so the one with the biggest number should be the most recent one, and the one that has your change. we always read from the most recent config file; so i wouldn't expect this to be a problem as i expect the 9 files to not have your thing as they're backed up versions of the config

nirmit commented 6 years ago

But the file that gets the update is not being used when a user clicks an email in the thread list.

As mentioned earlier, if you quit and reopen Nylas after making the change, it persists.

On Wed, Dec 13, 2017 at 11:41 PM Mike Seese notifications@github.com wrote:

The config.json files are timestamped backups. so the one with the biggest number should be the most recent one, and the one that has your change. we always read from the most recent config file; so i wouldn't expect this to be a problem as i expect the 9 files to not have your thing as they're backed up versions of the config

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nylas-mail-lives/nylas-mail/pull/166#issuecomment-351607168, or mute the thread https://github.com/notifications/unsubscribe-auth/AAaUJNFkqawGiV03Z8U3uePmdDRnh-keks5tAKb5gaJpZM4QaxzW .

mikeseese commented 6 years ago

Ah ok. I think you need to listen to the NylasEnv.config.onDidChange event to get the change in the config. Looks like what you're reading is just the initial config variable. Probably a similar example for this would be https://github.com/nylas-mail-lives/nylas-mail/blob/91d6514c97ebaa1cf8767be1eda31b14e27df427/packages/client-app/src/browser/system-tray-manager.es6#L59-L66