Adds Center Guides

[radiovisual]

This PR adds center guides to the rulers. You can toggle the visibility of the center guides on or off with the keyboard shortcut ⌘; (which is the same shortcut for toggling guide visibility in Photoshop), or you can use the Toggle Center Guides option in the lr menu.

The center guides are not visible by default.

There are also a few small formatting tweaks. To get the center guides to find the exact center of the ruler space, the margins on the top, right, bottom and left needed to be consistent, so a few of the UI elements on the ruler needed to change location slightly. This is what the tweaked ruler looks like (when the center guides are active):

[radiovisual]

My latest commit: only the focused ruler receives the center-guide shortcut ensures that the shortcut ⌘; only works on the active (focused) ruler, and not all the ruler windows (as it did previously). This is much better because it lets the users decided which rulers should show the center guides, instead of forcing them to choose between "on for every ruler", or "off for every ruler".

[radiovisual]

Sorry about mixing the "linting" pass in with this PR. I should have broken them up into two separate Pull Requests. I just starting making the spaces / tabs more consistent and got carried away. :sweat_smile:

[mikaa123]

Wow that was fast! Really cool :) I pulled your branch and ran a few tests. It works great! I show the center guides on the ruler I want without impacting others, which is really nice.

I don't have any problem with reorganizing the ruler's layout, however there's a little problem with small rulers: due to the min-width of the window, we can't make them smaller (and measure small values). It's not possible to go under 50px width and 25 height.

Got any idea on what we could do to fix that? Fantastic work btw!

[radiovisual]

Yes, I think I know the reason why the small rulers aren't possible, I will take a look at the code later on and fix it. As for the color red, I can change the color to be a light gray, but what do you think about letting the users set the color for the guides, and their color preferences get saved in the .linear file?

[mikaa123]

@radiovisual I think for now we should just hard-code the colors (I'll ask my designer friends for advices!). Later, if we allow custom CSS (themes) then they'd be able to change the color. So don't worry too much about finding the right color ;)

[radiovisual]

Ok, no problem. I will leave it red until you let me know what color values you prefer.

[radiovisual]

Ok, so it was easy to fix the "small rulers" problem. All that needed to be done was set the window min-width and min-height to 101px, which is the value that would allow the ruler to be a size of (0, 0) (pictured below):

[radiovisual]

but now there is some uneven padding on the right and left, so I will fix that now....

[radiovisual]

Ok, so I have changed the guides to default to lightgray for now, and I also fixed the problem where small rulers were getting weird padding behaviors on the right side. This is the result:

Lightgray Center Guides:

Borderless rulers allow for proper padding:

[radiovisual]

Ok, now the center guides are supported in the dark theme as well. When you merge this PR you can always change the default colors, but here is what they are as of right now:

Dark Theme & Light Theme Center Guides:

[mikaa123]

I ran into the following issue:

• Start the app
• A new ruler pops
• Click on it (so it has the focus)
• Lr icon > Toggle Center Guide won't toggle (BrowserWindow.getFocusedWindow() returns undefined)

It's kind of weird because sometimes it works, when there are a few rulers! I'll look into that a little bit later

[radiovisual]

I think its an issue with Electron. Yesterday I was experimenting with the BrowserWindow.getFocusedWindow() in relation to the blur and focus events and even inside the focus handler getFocusedWindow() would return undefined, but that was only in my experiments for disabling the context menu "Toggle Center Guides", I haven't had any problems with this Center Guides PR, so I will do this:

1. I will review my code to make sure I didn't introduce any bugs, and
2. If I find more strange problems with the getFocusedWindow() I will open an issue with Electron

[radiovisual]

When I run my local tests, it all works for me without any issue. So its weird how the getFocusedWindow() function is not playing very nicely.

I am going to setup an issue with electron based off of my experiments yesterday, and see if that will help us learn anything new.

[mikaa123]

Thanks! You did an amazing job. Here is what I'll do: I'll merge your PR and change BrowserWindow.getFocusedWindow() with:

• Have a lastFocusedRuler variable
• Anytime a ruler is created or focused, have lastFocusedRuler point to it

That way, no more problem with BrowserWindow.getFocusedWindow() :) Is that fine with you?

[radiovisual]

Yeah, if that works for you, then go for it! If there are any new problems, we will fix them! :smile:

[mikaa123]

Mergeeeed!

[radiovisual]

Just for future reference: I opened an issue with Electron, to see what is going on with getFocusedWindow()

[zulhfreelancer]

NICE!

[radiovisual]

@mikaa123 , just for future reference, here is the current workaround for the .getFocusedWindow() problem we were having (it is a known issue with Electron).

The current workaround is to simply wrap the functionality in setImmediate(). I just ran my own test, and this setup works perfectly:

win.on('blur', blurHandler);
win.on('focus', blurHandler);
// ...
function blurHandler() {
    setImmediate(function() {
        let focusedWindow = BrowserWindow.getFocusedWindow();

        if (focusedWindow) {
            console.log('focus!');
        } else {
            console.log('blur!');
        }
    });
}