Allow filling rectangles and ellipsis

jtheoof / swappy

A Wayland native snapshot editing tool, inspired by Snappy on macOS

MIT License

1.04k stars 39 forks source link

Allow filling rectangles and ellipsis #136

Closed flowln closed 1 year ago

flowln commented 2 years ago

Closes #120

This adds an option on top of existing brush types, to allow for filled shapes in addition to non-filled ones.

I tried to follow the surrounding style as much as possible, but let me know if something isn't quite right :sweat_smile:

foxpy commented 1 year ago

I like it and I would love to see it merged! However, I have two concerns:

maybe using toggle button is not the best way to implement this feature? I think checkbox would make more sense? not sure really;
I also wish there has been a feature to highlight region instead of filling it completely (maybe just by using opacity in color picker?), though it is out of scope of this PR.

flowln commented 1 year ago

I used a toggle button instead of a checkbox because that's what other "toggle-able" UI elements (they'd actually be more akin to radio buttons instead of checkboxes, but the logic still applies i think) in the UI currently use, so I followed for consistency.

For the highlighting, I would think either a field like 'Line Width' and 'Text Size', or a slider thing, could work for selecting an alpha value. As you've said though, that's not really in the scope of these changes, so maybe it could be done at some later point in time :slightly_smiling_face:

jtheoof commented 1 year ago

Thank you @flowln . It's looking good. I will squash all commits into one.