search

bitwarden / clients

Bitwarden client apps (web, browser extension, desktop, and cli).
https://bitwarden.com
Other
9.09k stars 1.2k forks source link

Zoom level not remembered on Ubuntu & Windows consistently #2623

Closed rohitarondekar closed 1 year ago

rohitarondekar commented 4 years ago

Describe the Bug

The Bitwarden desktop application on Ubuntu does not remember zoom settings. My laptop display has a high resolution and so I need to be able to zoom for the UI to be legible. Setting the zoom level manually at the moment every time I start the app.

Steps To Reproduce

  1. Start Bitwarden desktop application
  2. Login/Unlock account.
  3. Set zoom level using menu bar or keyboard shortcut.
  4. Close application.
  5. Relaunch application.

Expected Result

My previously set zoom level should be restored.

Actual Result

The zoom level is reset to default making everything small again.

Environment

Additional Context

There appears to be some memory retention of the zoom level because when I try to zoom again it goes one level higher then what I've set. So I'm guessing the zoom level is remembered but not applied at startup?

This might not be a priority as it's not the responsibility of the application to deal with HiDPI screens(?), but since there is the option to set zoom levels, being able to remember it would be a big help. I'd be happy to help with the code if it isn't very complicated to fix.

vachan-maker commented 4 years ago

Zoom level resets in Windows as well. I just tried this out.

vachan-maker commented 4 years ago

Some things I have noticed.

1.As said earlier, the application does not remember the zoom level and it resets when the app is restarted. 2.The zoom level increases and decreases by a different values for some reason after you have initially zoomed in or zoomed out. If you have already zoomed in then open the app again and then again zoom in, the zoom value increases by a lot.

  1. Next, zoom in->Restart the app->zoom resets->Minimize the window-> Zoom value changes
MahouShoujoMivutilde commented 3 years ago

Here is how it was fixed in standard notes (also electron app inside appimage)

https://github.com/standardnotes/desktop/pull/355

eliykat commented 3 years ago

We use window.main.ts to store and restore the window state, such as size and position. The existing patterns could be extended to remember zoom level as well. I've added it to the list of vetted issues, but PRs would be welcome if anyone wants to tackle it in the meantime.

skyler-shuman commented 2 years ago

I'd like to work on this if you can assign it to me.

anthonygedeon commented 2 years ago

@shumsk01 any updates on the fix? I can join and help out if needed

bonizzem commented 2 years ago

I have the same problem on windows, a workaround is to double click on window title maximizing the window, this reset the zoom as saved setttings. Doesn't do the same if click on maximize window button

mcint commented 2 years ago

I'm having this issue on macOS as well. Zoom levels are not remembered across application restarts. As of latest upgrade Version 2022.6.1 (3610).

dbosompem commented 2 years ago

@skyler-shuman have you been able to make progress with this? If not, I can unassign , and be open to any PR. @anthonygedeon you could also let me know if still interested to work on this, and it would be assigned to you.

skylerahuman commented 2 years ago

My apologies for not communicating better. I had some issues that limited my ability to work on the project.

My initial idea for a solution was to extend the WindowState to include the webPreferences -> zoomState. However, there is not currently a use of webPreferences in updateWindowState and I'm unsure if it would be a good idea to add one.

An alternative solution, similar to what occurred in standardnotes would be to introduce a zoomManager dedicated to handling zoom. However, this doesn't seem like the best solution given BrowserWindow already has a webPreferences object which stores the zoomState and can restore it when being instantiated.

I am also confused why updateWindowState takes the argument configKey but the only configKey ever passed is mainWindowSizeKey. It appears to imply there are multiple possible WindowStates however, the TODO in the WindowState class implies there should only be a singular client-specific global state.

Finally, why is windowStates's type declaration not for values of the class WindowState? After further research, I see this was done to prevent TypeScript's abstraction from preventing access of WindowState's properties.