thamara / time-to-leave

Log work hours and get notified when it's time to leave the office and start to live.
http://timetoleave.app
GNU General Public License v3.0
463 stars 271 forks source link

Fix #877: Dialogs open in wrong monitor in multi-monitor configuration #897

Closed SarahRemus closed 2 years ago

SarahRemus commented 2 years ago

Related issue

Closes #877

Context / Background

At the moment, the Preferences dialog and the Waiver Manager dialog are always opening centered in the main monitor instead of the secondary monitor, no matter where the main TTL window is. Also, if the TTL window is, for example, in the right corner of the screen, the dialog window still opens up in the center of the main monitor.

What change is being introduced by this PR?

I fixed this issue by specifying concrete x and y values for the dialog window related to the bounds of the mainWindow. This way, the dialog window always opens up in the upper left corner of the TLL window, no matter where the window is currently located. If it is a design choice to always open the dialog window in the center, I can further change my PR.

image

How will this be tested?

Steps to test this:

  1. Open TTL
  2. Move it to a secondary display
  3. Click on 'Preferences'
  4. See it open on the current display, in the upper left corner of the TLL window
araujoarthur0 commented 2 years ago

Cool, thanks! I think I prefer the windows centered on the TTL window, if possible, to give the idea you should work on them and close before going back to the main window.

araujoarthur0 commented 2 years ago

What do you prefer, @tupaschoal and @thamara ?

tupaschoal commented 2 years ago

I don't have a strong preference. Centered or not, but positioned relative to the main window is already a lot better than the current behavior

SarahRemus commented 2 years ago

I can also implement it, so that the dialog is always in the center of the main TTL window. At the moment I have some difficulties with running TTL locally that I have to fix first, then I will continue working on this issue.

SarahRemus commented 2 years ago

I changed by PR so that the dialog window always appears in the center of the TTL window.

araujoarthur0 commented 2 years ago

I'm thinking whether we should create a function on window-aux that returns these calculations. What do you think @araujoarthur0 ?

I agree.

SarahRemus commented 2 years ago

I'm currently working on implementing this function. However, I'm not sure if the window-aux.js file is the right place for the function. It appears to me that all functions implemented in this file are only used during rendering. If I try to import functions from window-aux.js to windows.js I get the following error: Cannot destructure property 'BrowserWindow' of 'remote' as it is undefined. I think we should create a new file named dialog-coordinates-aux.js and define the coordinate functions there. Then we can import them to windows.js (for waiver dialog) and menus.js (for preference dialog).

What do you think about that?

araujoarthur0 commented 2 years ago

Can you implement it in windows.js instead? Or creating a window-aux.js in the main folder instead of the renderer.

codecov[bot] commented 2 years ago

Codecov Report

Merging #897 (644cda5) into main (f9a9f10) will increase coverage by 2.22%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #897      +/-   ##
==========================================
+ Coverage   71.45%   73.67%   +2.22%     
==========================================
  Files          17       17              
  Lines        1755     1755              
  Branches      278      278              
==========================================
+ Hits         1254     1293      +39     
+ Misses        501      462      -39     
Impacted Files Coverage Δ
js/classes/BaseCalendar.js 58.01% <0.00%> (+2.67%) :arrow_up:
js/classes/FlexibleMonthCalendar.js 60.41% <0.00%> (+4.16%) :arrow_up:
js/classes/FlexibleDayCalendar.js 65.01% <0.00%> (+6.36%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

tupaschoal commented 2 years ago

\changelog-update Message: Fix [#877]: Dialogs open in wrong monitor in multi-monitor configuration

tupaschoal commented 2 years ago

Thanks for the work @SarahRemus !