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

Allow the ToolPicker to have a scrollbar #258

Merged
merged 1 commit into from Nov 14, 2021

Conversation

precla
Copy link
Contributor

@precla precla commented Nov 8, 2021

The ToolPicker is never fully visible when the kImageAnnotator window is smaller than the ToolPicker area.
Occurs especially when used embedded within an program, like Spectacle, which makes it almost
impossible for the user to know if there are more tools to choose from. Instead of going by pure
luck/intuition and resizing the main window, in this example the window of Spectacle, showing a scrollbar
when the window is too small to show all tools by default gives a hint to the user that there are more tools available.
The scroll bar would be only shown when needed, not always :)

1108_213417

How it currently looks within Spectacle:

1108_213657

@DamirPorobic
Copy link
Member

DamirPorobic commented Nov 9, 2021

@precla thanks for providing this PR, I like the idea but there is a problem, currently you can do something like this:
image

To have the icons in two columns, I know some users like to have it that ways as it resamples the legacy layout.

Now cannot do that anymore as you see the scrollbar always:

image

Is there a way to combine both features? Try multiple columns and if you've used up all column space and some icons still hidden then show scrollbar?

(btw I like you console prompt, what are you using? Is that plain zsh?)

@DamirPorobic
Copy link
Member

Oh and why is the Spectacle window so small? I thought they have a separate window for annotation? That one seems indeed to be to small for all the controls.

@precla
Copy link
Contributor Author

precla commented Nov 9, 2021

First the important thing: console is KDE's Konsole, using oh-my-zsh with USE_POWERLINE="true" and manjaro-zsh-configuration :)

Hm, how to have icons in two columns? Looks like I missed the piece of code that enables that.

And yes, Spectacle keeps the Annotator inside the current Spectacle window, using the current window size of Spectacle.

@DamirPorobic
Copy link
Member

First the important thing: console is KDE's Konsole, using oh-my-zsh with USE_POWERLINE="true" and manjaro-zsh-configuration :)

Looking nice, I'll have to try that out.

Hm, how to have icons in two columns? Looks like I missed the piece of code that enables that.

Just resize to the right (in this case but the widget can be positioned on left, right, bottom or top so basically resizing in all direction). It should be trying to got to the right size as far as possible, then switch to next row and so on:
image

The magic is done by the FlowLayout.cpp class (src/widgets/misc).

@precla
Copy link
Contributor Author

precla commented Nov 9, 2021

Thanks, just tried it in Spectacle. Tho, would be nice if there is a function call that would do that, instead of the user having to resize it. Not every user will notice this.

I will take a closer look into this on the weekend when I have more time, maybe we can have both scrollbar and this resize with multiple columns.

@DamirPorobic
Copy link
Member

Thanks, just tried it in Spectacle. Tho, would be nice if there is a function call that would do that, instead of the user having to resize it. Not every user will notice this.

Problem is you don't know how far to go, you could always assume to go with two rows but ksnip for example goes with one. Or someone would like four columns and three rows. Tastes are different.

I will take a closer look into this on the weekend when I have more time, maybe we can have both scrollbar and this resize with multiple columns.

That would be lovely. Let me know if you need anything. :)

@precla
Copy link
Contributor Author

precla commented Nov 13, 2021

Found the solution:
https://forum.qt.io/post/119409

Applied & tested it, almost as you need it :) . When the Tool Selection widget is resized so that 3x3 icons fit, one icon is missing (the sticker) and there is no scrollbar :s

This one was missing:

https://github.com/ksnip/kImageAnnotator/pull/258/files#diff-2bd0d8bd582468496f4c04a11e3583390064df832bd6125195548f06bfa1082bR29

@DamirPorobic
Copy link
Member

DamirPorobic commented Nov 14, 2021

Looking odlično :)

Merging it into 0.5.3 so it get's delivered with the next patch. Thanks for providing this enhancement/fix!

@precla
Copy link
Contributor Author

precla commented Nov 14, 2021

Looking odlično :)

Merging it into 0.5.3 so it get's delivered with the next patch. Thanks for providing this enhancement/fix!

Zahvaljujem :)

Have you tested it? Because of the weird bug with 3x3 and one icon missing :D

@DamirPorobic
Copy link
Member

Have you tested it? Because of the weird bug with 3x3 and one icon missing :D

Yes, I can reproduce it, also with 4x2 when two icons are in the last row but that's ok for now, it's minor issue, at least it's better then it was.

One more think, what do you think of removing the second border? This widget is the only one with two. mScrollareaToolPicker->setFrameShape(QFrame::NoFrame);

@precla
Copy link
Contributor Author

precla commented Nov 14, 2021

mScrollareaToolPicker->setFrameShape(QFrame::NoFrame);

Yeah, let's do it. Looks much better :)

The ToolPicker is never fully visible when kImageAnnotator is used embedded within an program,
like Spectacle. Which makes it almost impossible for the user to know if there are more
tools to choose from. Instead of going by pure luck/intuition and resizing the main window,
in this example the window of Spectacle, showing a scrollbar by default gives a hint to the
user that there are more tools available.
@precla
Copy link
Contributor Author

precla commented Nov 14, 2021

Added within this MR

@DamirPorobic DamirPorobic merged commit 9698666 into ksnip:master Nov 14, 2021
@DamirPorobic
Copy link
Member

Merged, thanks again for providing the PR :)

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