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

Swatches can be set to fit in widget - if possible #1264

Merged
merged 10 commits into from Feb 27, 2020

Conversation

davidlamhauge
Copy link
Contributor

@davidlamhauge davidlamhauge commented Sep 16, 2019

David_Pencil2d_20190916_17_23_16
A small enhancement to the Palette Widget.
In stead of using the drop down menu and choose between three fixed sizes (14, 26 and 34 pixels), you can use the slider and get the size you want, from 14 to 32, in steps of 2.
Then you can (almost) always fit your palette in the window with ease.

@scribblemaniac scribblemaniac added 🔹 Minor PR (only one reviewer required) Color Palette Enhancement labels Sep 17, 2019
Copy link
Member

@MrStevns MrStevns left a comment

LGTM code wise... still not sure about it's inclusion.
@scribblemaniac didn't you have a comment regarding this too? I seem to recall a discussion on discord while I was on vacation but I can't seem to find it...

I personally don't see the need to have a slider to change the swatch size, not on the widget itself anyhow, currently there's enough space for it that it doesn't matter but in the future there might be other things we'd like to put there instead.

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Sep 28, 2019

@candyface : @scribblemaniac had the same reservations that you have. He wouldn't mind a slider, but it shouldn't take up place in the widget, since the space could be better used with other things.
I haven't had time to look at it yet, but I will investigate some possibilities I have in mind, and if that doesn't work out, I'll close the issue.

@Jose-Moreno
Copy link
Member

Jose-Moreno commented Sep 28, 2019

@scribblemaniac @candyface @davidlamhauge I see this PR as solving a broader problem which is improving the user experience upon modifying swatch size which is interrupted with multiple clicks (and having only 3 choices whereas David's has 6, and could probably add more levels).

I personally don't see a problem with the slider, but if you don't want to include it as a slider, then in the context of the palette, we could map the existing set of actions from this PR, using associated keyboard shortcuts. Perhaps something like +/- , scrolling the mouse wheel on a 3 mouse button or using the "ring" button on a graphics tablet or specialized hardware.

However we'd have to alter the "small, medium large" buttons and replace them for a "smaller (-), larger (+)" set of buttons instead and possibly have a palette preference to adjust the maximum size allowed for the palette swatch size "steps".

My only problem is that without the slider or any other visual solution, it's not "obvious" what you can do and this is a bigger problem. We know that quite some people don't even read the menus, how can we expect them to find the size option buried in the "miniature" palette options menu? While this is not a PR to address the app UX design, I think it's worth to note that it is intuitively considering this point as well.

People basically don't use the features Pencil2D has because they don't know they exist!, and this is not a problem with mere documentation lacking as one user chanted back then, this is a problem with discoverability of the features. They use what they can see, which is why we get a dozen questions weekly asking the same thing.

So if possible I'd prefer a visual solution for this enhancement / UX focused PR. If that can't be done then the next best thing is having a shortcut feast 😋

@MrStevns
Copy link
Member

MrStevns commented Sep 28, 2019

@scribblemaniac @candyface @davidlamhauge I see this PR as solving a broader problem which is improving the user experience upon modifying swatch size which is interrupted with multiple clicks (and having only 3 choices whereas David's has 6, and could probably add more levels).

It's interrupting the workflow maybe once or twice, if at all and then you don't have to think about it again. I've used it a couple of times myself. Is this something you change often and is there a special need for more fine grained control of the sizing?

I implemented this feature because I like seeing a lot of swatches always available, especially if the interface takes up a lot of whitespace. I only use it with grid mode and changing size is something I rarely do.

I personally don't see a problem with the slider, but if you don't want to include it as a slider, then in the context of the palette, we could map the existing set of actions from this PR, using associated keyboard shortcuts. Perhaps something like +/- , scrolling the mouse wheel on a 3 mouse button or using the "ring" button on a graphics tablet or specialized hardware.

Hiding the functionality behind shortcuts only is not good idea either, it should either and easy to find or visible or not available at all, we're not blender (before 2.8) :P but there's also another problem or limitation that would make it difficult to implement.
The shortcut system does not support binding to the mouse buttons, so if we bind the mouse wheel or a mouse button to a palette feature like increasing or decreasing size, then you wouldn't be able to re-bind it to something else, since we can't bind to anything other than standard keyboard button atm.

However we'd have to alter the "small, medium large" buttons and replace them for a "smaller (-), larger (+)" set of buttons instead and possibly have a palette preference to adjust the maximum size allowed for the palette swatch size "steps".

This could be done but I'd argue that the functionality would be worse, since you now have to select"more"->"increase size" multiple times to get to a certain size, which would definitely make for a crappier UI/UX. Alternatively we could have a "set size" button hidden in the miniature button, where a dialog appears and you can set the size but I still don't see the need for it.

My only problem is that without the slider or any other visual solution, it's not "obvious" what you can do and this is a bigger problem. We know that quite some people don't even read the menus, how can we expect them to find the size option buried in the "miniature" palette options menu? While this is not a PR to address the app UX design, I think it's worth to note that it is intuitively considering this point as well.

That's a problem with the miniature or "more settings" button though and I think that should be addressed in separate discussion.

People basically don't use the features Pencil2D has because they don't know they exist!, and this is not a problem with mere documentation lacking as one user chanted back then, this is a problem with discoverability of the features. They use what they can see, which is why we get a dozen questions weekly asking the same thing.

True although it's also a lack of trial and error on the user, pencil2d is still not big nor complicated compared to something like KRITA, Gimp and the like. Having an additional of x amount of buttons always available won't make that better, only clutter the interface further, thus making it seem more complicated than it is.

So if possible I'd prefer a visual solution for this enhancement / UX focused PR. If that can't be done then the next best thing is having a shortcut feast 😋

Some things ought to be harder to get to or be hidden away, to make the interface feel as light as possible. It's a choice that I would make because the feature itself imo. lies in the bucket of "Quality of Life" or "Nice to have" things. I don't consider it a crucial component for an animator and would therefore rather have other things being easy to get to and take up visual space.

@scribblemaniac
Copy link
Member

scribblemaniac commented Sep 28, 2019

I agree with everything @candyface has said. In fact, and I think I hinted at this on Discord, I don't think it needs to be accessible from the main window at all. Something like the display size of the palette seems to me like a preference and the preferences would be the first place I would look for it if I did not already know where it was.

This isn't something that I could see people changing often enough for it to need to be constantly accessible, let alone constantly visible. However there is one exception I could see: users may have different preferences for the size of the grid and list modes. In which case each time they want to switch views, they will also need to switch sizes. I think the solution to this is to separate the grid and list mode sizes into two different settings in the preferences window.

With list mode button, it is not clear what it does without pressing it, and it is admittedly a little bit small out out of the way. However the same can be said of the slider, you don't know what it's going to do until you try modifying it. The only thing this changes is to make it take up more space in the hope of attracting more attention.

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Sep 29, 2019

swatch_resizing
I have removed the slider, and added one menu point, that allows the swatches to fit into the widget - if there is room. Swatch sizes will be from 19-36 in size.
The user will still be able to set swatch sizes to Small, Medium and Large.
While debugging, I discovered that if the swatch size came below 19, it was only the icon and not the swatch itself that became smaller. For this reason I've set the minimum swatch size to 19.

@davidlamhauge davidlamhauge changed the title Swatch slider Swatches can be set to fit in widget - if possible Sep 30, 2019
Copy link
Member

@MrStevns MrStevns left a comment

Looks good to me, my only gripe is that the new fit to size feature only works for list mode. If it's not going to have same behaviour for grid mode, then the action should be hidden when grid mode has been selected.

@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Oct 3, 2019

In fact it also worked in grid mode, but the calculations were based on size in list mode, so the window should be unnormally high before size changed in grid mode.
I have fixed this bug now - as good as I can. It is hard to calculate the width of the swatches, because padding is irregular, and then there is the scrollbar. I think it works fine now. I've tested it with many and few swatches, and I think it is "As good as it gets" ;-)

@MrStevns MrStevns added this to the 0.6.5 milestone Nov 10, 2019
Copy link
Member

@MrStevns MrStevns left a comment

I approve of the functionality, seems to work fine 👍
Now we just gotta polish the code a bit more and then it's good to go.

app/src/colorpalettewidget.cpp Outdated Show resolved Hide resolved
app/src/colorpalettewidget.cpp Outdated Show resolved Hide resolved
app/src/colorpalettewidget.cpp Outdated Show resolved Hide resolved
app/ui/colorpalette.ui Outdated Show resolved Hide resolved
Copy link
Member

@scribblemaniac scribblemaniac left a comment

I still would be in favour of a preference for more fine-grained controls, but this is fine too. One issue though: fitSwatchSize should also be called on palette import.

app/src/colorpalettewidget.cpp Outdated Show resolved Hide resolved
@davidlamhauge
Copy link
Contributor Author

davidlamhauge commented Nov 14, 2019

Requests from @scribblemaniac handled.

@MrStevns MrStevns requested a review from scribblemaniac Dec 8, 2019
@MrStevns
Copy link
Member

MrStevns commented Feb 27, 2020

I've resolved merge conflicts and the remaining issues has been reported fixed.
Merging.

@MrStevns MrStevns merged commit f3ce5d9 into pencil2d:master Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Color Palette Enhancement 🔹 Minor PR (only one reviewer required)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants