Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General improvements to pinned image scaling behaviour #3428

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

schwubdiwub
Copy link

I changed a few things, all involving the scaling behaviour of pinned images.

Foremost, I changed the formula for calculating the change of scale. Now the rate of change scales with the current image size. Before, for example, an image with original size of 100x100 would change size at another speed than an image with original size of 400x400 that had been downsized to 100x100. Originally bigger images changed their size faster than originally smaller images.
Another thing about scaling speed I changed is, that it is now possible to hold ctrl / shift while scrolling to increase / decrease the scaling speed by 5 times. Since this involves hotkeys, this might be something to discuss?

Then, when creating a screenshot which size was smaller than the hard-coded minimum image size of 100, scaling would cause the pinned image to make a jump to hard-coded min size dimensions and prevented to scale in the range of original size and hard-coded min size. Now the lower bound of scaling is set to whatever is smaller, the original size or the hard-coded min size.

Lastly, I fixed the behaviour when scaling down to minimum image size and then keeping scrolling down. This caused the internal scaling factor for the image to further decrease, which in turn caused that trying to scale up would not react immediately, but you had to keep scrolling up until the internal scaling factor reached its value equivalent to min image size and after that point the image size started to increase again. Now the internal scaling value is limited as well when the image size is limited by the min image size.

If there is anything wrong about my way of contributing, please let me know, since this and another pull request are my first contributions to this project. Feedback is appreciated 😄

schwubdiwub added 4 commits December 3, 2023 00:23
…e scaling

Pinned images had a fixed min size which, in case the pinned images original size was smaller, prevented to zoom back to original size after zooming past the fixed min size. Now in case the original image size is smaller the image size is used as a lower bound for scaling
Rapidly changing the size of a pinned image was not possible and bigger size changes involved heavy scrolling, now 'ctrl + mouse wheel' has an increased zooming speed (5x) for those cases
Similarly more precise changes of size where not possible, 'shift + mouse wheel' now makes this easier with 0.2x zooming speed
…in size

When decreasing the size of pinned images, the scaling factor would not stop decreasing when the min image size has been reached. This caused that, after scrolling down past reaching min size, scrolling up would not increase the image size immediately, because the internal scale factor had to reach min size again. Now scrolling up after reaching min size will in all cases increase the image size.
@mmahmoudian
Copy link
Member

Thanks for the contribution👍. We generally prefer to discuss such big changes first before implementing them. This procedure would have clarified some of the questions you yourself also posed.

Regardless, here are my comments (we should wait for other devs' comments too):

Originally bigger images changed their size faster than originally smaller images.

Yes, this was something that I also observed, and was also a bit hurting my workflow as well.

Then, when creating a screenshot which size was smaller than the hard-coded minimum image size of 100, scaling would cause the pinned image to make a jump to hard-coded min size dimensions and prevented to scale in the range of original size and hard-coded min size. Now the lower bound of scaling is set to whatever is smaller, the original size or the hard-coded min size.

This is nice, but it creates a problem: losing the pinned window. Considering that the pinned image windows does not have decorations, if the size is small, it would be very easy to lose it. I'm not against your change, but I think we should have a mechanism to help user find/locate the window if it is very small. For instance we can set the key binding like = to always change the size back to the original size unless the original size is smaller than 100 pixels which then we actually increase the size to 100 pixels.

Now the internal scaling value is limited as well when the image size is limited by the min image size.

I like this change too. I have also observed this behavior in my test as well, but it was so rare that I thought it is not hurting the user's experience.

@mmahmoudian
Copy link
Member

There's also one point that you have used hard-coded key bindings but there is no UI representation of them. I would suggest adding them to the right click menu as well so that the user can learn/discover the key bindings from that menu.

@schwubdiwub
Copy link
Author

Yeah, I read the contribution guidelines, but I was not sure where to draw the line between incremental improvements and bigger changes. And as I was already fiddling with the code, I took this route to find out more about the process.
I can create an issue for the proposed changes. Or we can keep the discussion here, since we are already discussing?

This is nice, but it creates a problem: losing the pinned window. Considering that the pinned image windows does not have decorations, if the size is small, it would be very easy to lose it. I'm not against your change, but I think we should have a mechanism to help user find/locate the window if it is very small. For instance we can set the key binding like = to always change the size back to the original size unless the original size is smaller than 100 pixels which then we actually increase the size to 100 pixels.

That is a good point. And I like the key binding idea, but then there is the problem of keyboard focus. If the widget is already tiny, that the user can't locate it, how can the user control that the use of the key binding will affect the lost widget? And a global key binding that affects all widgets is a no-go in my opinion, since this will destroy an already assembled collection of pinned images and will possibly create clutter on the screen.
I have seen, in #2984, that there is a desire to make pinned images minimizable again. This could help user in case of lost widgets as well.
Another idea is to create a separate "pinned images managing" dialog, to handle this case and possibly add other features, too. But at the same time, I think this will leave the intended scope of this project 😆

There's also one point that you have used hard-coded key bindings but there is no UI representation of them. I would suggest adding them to the right click menu as well so that the user can learn/discover the key bindings from that menu.

That is a good point, too. Right now, the implementation does not align with the behaviour of checked actions found in such menus. So it would be possible to add an action or two that do nothing when clicked, but are hinting the user at the key bindings. But at the same time, it could be confusing for the user to have actions that actually do nothing. Maybe add a tool tip to the widget, which serves this purpose?
Changing the behaviour of the hotkeys to align with the behaviour of checked action would be possible, too. But I think this will decrease the usability for this feature.

@mmahmoudian
Copy link
Member

I can create an issue for the proposed changes. Or we can keep the discussion here, since we are already discussing?

Let's discuss here 😊

If the widget is already tiny, that the user can't locate it, how can the user control that the use of the key binding will affect the lost widget?

Yes, you are correct. We should find a solution for this issue. I like the idea of a panel for pins, but that might be overkill. Maybe we can have all the pinned images to become 100x100 and get arranged in a grid? Although I have a feeling that it is a bad idea because of Wayland and tiling window managers. Making them minimizable is good since Atl-tab can let the user to cycle through windows as well.

it could be confusing for the user to have actions that actually do nothing. Maybe add a tool tip to the widget, which serves this purpose?

Perhaps we can have a window show keybindings when ? is pressed. Like in a separate window (normal window with decorations and etc.)

@schwubdiwub
Copy link
Author

Maybe we can have all the pinned images to become 100x100 and get arranged in a grid? Although I have a feeling that it is a bad idea because of Wayland and tiling window managers.

And, as well, in case someone has a few pinned images arranged in a certain layout to fit over another window, the auto arrangement into a grid would destroy the user's layout. So making them minimizable is the better solution in my opinion. And it handles an open issue at the same time.

Perhaps we can have a window show keybindings when ? is pressed. Like in a separate window (normal window with decorations and etc.)

I really like this idea, since there are other hidden hotkeys for changing the opacity that are not visible to the user (at least I didn't see where). We can put all those key bindings in the dialog and maybe add a few more for the other right click options, like arrow keys to trigger left and right rotation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants