@sindresorhus updated 👍

I still see some weird behavior even with this PR. I'm gonna include my feedback here, but I would be happy to move some of the points over to a new issue if you feel they're out of scope for this PR:

• The cropper aspect ratio dropdown should not open right under wherever the mouse is, but rather open like a normal dropdown menu always placed underneath the input.
• The same as above applies to the ... menu.
• In the width/height input, if I use the down arrow key to go down from 10, it continues even to negative numbers. It should stop at 0.
• The width/height input needs a minimum value. It makes no sense to support dimensions like 100 width and 0 height. I suggest 20 be the minimum as 20x20 is the smallest we can make the cropper without the drag handles overlapping.
• Selecting an aspect ratio should active the width/height lock. Otherwise, what's the point of selecting aspect ratio if it's not enforced when you change width or height.
• Putting the crop rectangle right at the right edge and then changing the width is not allowed and makes the width input shake. This should be allowed and the crop rectangle should just expand outwards to the left.
• Similar as the above, if I make the crop rectangle 100x200 and then put it at the right edge, then click the swap button twice, the size has changed. The size should be the same as it was
• With an aspect ratio set and the width/height lock on, if I click and hold the top-left drag handle and drag the mouse over to the top-right drag handle the cursor changes and it starts to resize from the top-right instead. The drag direction should not change during a drag operation.

These are good points. We've talked about most of them in the past and I've worked on some but last time I did was when I first started the re-write. I'll work on it a bit more, see how it goes, and if it seems like it's going to be a lot of extra work, I'll see about splitting into two PRs

This also fixes #539 now

@karaggeorge In #528 (comment), can you check the things you've fixed?

@sindresorhus it's all of them (hopefully) 😄 I went ahead and checked them though

Also, for the ratio menu I used the itemPositioning option of the menu api to show the selected one under the cursor. I saw it was osx specific, so I'm guessing is standard osx practice?

Also, for the ratio menu I used the itemPositioning option of the menu api to show the selected one under the cursor. I saw it was osx specific, so I'm guessing is standard osx practice?

Not cursor, but yes it's common practise to show it under the element that triggered the menu. It needs to be better aligned though. When the menu is open, the active menu item should be vertically centered to our white dropdown widget.

Example:

Notice how the active menu item is vertically centered with the dropdown button.

I'm getting this error every time I click the Kap menu bar icon:

Selecting an aspect ratio should active the width/height lock. Otherwise, what's the point of selecting aspect ratio if it's not enforced when you change width or height.

Doesn't look like this is done. If I select an item in the aspect ratio dropdown, the lock button is not highlighted.

I can't recreate the issue @sindresorhus is having when clicking the menubar icon, I can however confirm that selecting an spect ratio does not lock the dimensions, which I think it should.

Overall this is such a massive improvement, after testing the branch for a few days I haven't had a single issue with it and it feels great to use. The only thing I might add is for the inputs to only allow number input, however that's not really common practice. In a future update a nice to have would be the ability to do simple math in the inputs, e.g. 64+64 would change the input to 128 on enter/blur.

@karaggeorge Can you fix the merge conflict?

I'm no longer getting the focus error when clicking the menu bar icon. Weird...

The only thing left is #528 (comment)

@sindresorhus I'll fix the merge conflicts when I get some down-time. About the other comment, I tried to get it as close to centered as I could. Bringing it one more pixel makes it even worse. Any idea of a consistent way to do it?

About the other comment, I tried to get it as close to centered as I could. Bringing it one more pixel makes it even worse. Any idea of a consistent way to do it?

Hmm, actually it looks enough for me now. Must have been something weird when I tried earlier as I could swear it was not aligned then.

Should be all good. Also, I found a bug where the validation was pulling the primary display screen's size instead of the one we sent through ipc. So in external displays, the cropper was limited to the primary display size. Should be fixed now

Looks great!