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

Add additional useful widgets to the status bar #1442

Merged
merged 8 commits into from
Jul 24, 2021

Conversation

J5lx
Copy link
Member

@J5lx J5lx commented Aug 26, 2020

This adds a bunch of additional information to the status bar in preparation for #1151 – in particular the current tool, whether the current file has been modified, the current layer and a zoom control inspired by Krita’s. Originally this work also contained a timecode, but I removed that part since it has been submitted separately in #1423 in the meantime. I can take care of the conflicts once either PR gets merged.

In addition to the status widgets I also added some basic tool help, inspired by GIMP and Blender. I hope this will help answer some of the basic questions people ask, such as “How do I rotate the view?” or “How do I move my selection?” or “How do I finish a polyline?”. Further suggestions for improvement are welcome!

Screenshot_20200826_193133

(This screenshot still shows the timecode, but as I said that is not part of this PR)

@J5lx J5lx added Enhancement UX Related to the way users interact with the program labels Aug 26, 2020
app/src/mainwindow2.cpp Outdated Show resolved Hide resolved
@chchwy
Copy link
Member

chchwy commented Aug 27, 2020

Generally, I like this status bar. Since it's getting complicated, do you consider to move all this status bar related code to an individual file?

@J5lx
Copy link
Member Author

J5lx commented Aug 27, 2020

I’ll look into it!

@J5lx
Copy link
Member Author

J5lx commented Aug 28, 2020

I’ve moved the status bar code out of the main window and optimized the zoom update slot.

@J5lx J5lx force-pushed the features/statusbar-enhancements branch 2 times, most recently from 6f7fbfb to 04745e4 Compare August 28, 2020 17:52
@MrStevns
Copy link
Member

MrStevns commented Aug 29, 2020

Here's a small visual review, I haven't looked at the code yet, this is just me testing the new stuff.

The space from the tool icon to the left window edge compared to the right window edge doesn't seem to be equal. At least I'd like a bit more space from the icon to the edge.
image
image

The zoom slider has ticks but they don't seem to do anything, it's impossible to go back to 100% without typing it into the spinbox, is my experience.
image

The "project has been modified" star (*) is too cryptic and imo. may not be needed since it's already written in the window title or the close dot on mac os. Alternatively it could say "Unsaved".

@J5lx
Copy link
Member Author

J5lx commented Aug 29, 2020

Thanks for the feedback. I don’t think it’s possible to make the icon perfectly equidistant to the edges because the exact spacing depends on the interface style in use, but I can certainly add some horizontal padding at least.

Qt doesn’t provide any functionality to have the slider snap to the ticks specifically, it seems to be a purely visual indicator. Since I don’t have any issue putting the slider back to exactly 100% on my system, it most likely depends on other factors as well to a degree. I’ll try decreasing the range / increasing the step interval a bit to alleviate the problem, but it will also reduce the granularity.

The modification indicator is included in preparation for #1151 (full-screen mode), actually it was first suggested over there. I don’t want that part to consume too much space, so I’ll look for an icon to replace the asterisk.

@MrStevns
Copy link
Member

MrStevns commented Aug 29, 2020

I don’t think it’s possible to make the icon perfectly equidistant to the edges because the exact spacing depends on the interface style in use, but I can certainly add some horizontal padding at least.

Adding a bit padding is fine, it doesn't have to be the same.

Qt doesn’t provide any functionality to have the slider snap to the ticks specifically, it seems to be a purely visual indicator. Since I don’t have any issue putting the slider back to exactly 100% on my system, it most likely depends on other factors as well to a degree. I’ll try decreasing the range / increasing the step interval a bit to alleviate the problem, but it will also reduce the granularity.

Ah I see, in that case why not use the standard slider GUI eg. remove the "setTickPosition" for QSlider, since we don't use it anywhere else?

The modification indicator is included in preparation for #1151 (full-screen mode), actually it was first suggested over there. I don’t want that part to consume too much space, so I’ll look for an icon to replace the asterisk.

I get that but without much context, if the user doesn't understand the symbol, then it's not of much use. I don't think writing "unsaved" or something similar is that bad. If it's meant for fullscreen, then there should be enough space. Alternatively we could have that indicator somewhere else.
image

@J5lx J5lx force-pushed the features/statusbar-enhancements branch from 04745e4 to 011e5d0 Compare August 29, 2020 18:08
@J5lx
Copy link
Member Author

J5lx commented Aug 29, 2020

I’ve experimented with the changes I mentioned above, please give it a try. For the padding, I’ve used 3px for now, which is about the maximum amount that works before it starts looking weird on my system too. However, if that is still too little for your system, please do experiment with the amount a bit yourself and let me know what would work best.

remove the "setTickPosition" for QSlider, since we don't use it anywhere else?

It’s true that we don’t use them much elsewhere, but most of our existing sliders also don’t really have “semantically relevant” values like 100% for the zoom slider. I’d prefer keeping the ticks even if they’re only visual. That said, I don’t feel particularly strongly about it.

I don't think writing "unsaved" or something similar is that bad

What I’m kinda worried about is users with smaller screens, I don’t want another “overlapping panels”-style issue like the one we have right now. David’s timecode implementation in its current state can also take up a good amount of space. For now I abused the save/saveas icons for this. It’s kinda hacky but I think it actually conveys the meaning surprisingly well IMO, let me know what you think.

@J5lx J5lx force-pushed the features/statusbar-enhancements branch from 011e5d0 to e64159d Compare August 29, 2020 18:35
@MrStevns
Copy link
Member

MrStevns commented Aug 30, 2020

I’ve experimented with the changes I mentioned above, please give it a try. For the padding, I’ve used 3px for now, which is about the maximum amount that works before it starts looking weird on my system too. However, if that is still too little for your system, please do experiment with the amount a bit yourself and let me know what would work best.

It's fine now 👍

It’s true that we don’t use them much elsewhere, but most of our existing sliders also don’t really have “semantically relevant” values like 100% for the zoom slider. I’d prefer keeping the ticks even if they’re only visual. That said, I don’t feel particularly strongly about it.

Hmm right, that's true.. alright then let's just keep it.

What I’m kinda worried about is users with smaller screens, I don’t want another “overlapping panels”-style issue like the one we have right now. David’s timecode implementation in its current state can also take up a good amount of space. For now I abused the save/saveas icons for this. It’s kinda hacky but I think it actually conveys the meaning surprisingly well IMO, let me know what you think.

My personal problem with using an icon without context is that, it's difficult to understand whether it conveys a state or an action, my first intuition is that it's clickable and works as a button because that is how we use the save icon elsewhere.

Instead of abusing the save and save-as icons, I think it would be better to use disabled grayed out icon that Qt generates for us.

Off:
image

On:
image

The existing icons should imo. not be used to convey multiple things, unless the result is the same, as the user will not necessarily know what to expect. What I'm suggesting above is not ideal either but I do think it's better than using the "save" together with "save as".

@MrStevns
Copy link
Member

MrStevns commented Aug 30, 2020

Btw. while I know that #1151 mentions

Implement on-screen current layer name

I don't see why we should have it displayed there, isn't the assumption that the timeline is always displayed somewhere?
@Jose-Moreno can you chim in here, since you're the one who suggested it?

I think it's a good idea to implement actions and indicators that are not accessible in fullscreen, but whether onion skin is on or off or what layer you're working on, this information can be found elsewhere.

I'm saying this because I don't think the status bar should contain too much information, it shouldn't be a place where we put whatever we deem necessary, for the sake of always having it available, since as @J5lx mentions, the more we add to the status bar, the more the min. width will grow, plus it creates duplicate UI for no reason.

@Jose-Moreno
Copy link
Member

@candyface @J5lx Hey, sorry this PR went over my feed, I didn't know you guys were working on this.

Uhh I suggested those points from 1151 for an actual overlay, not a toolbar description. Something like this (top left portion of the image) but with better design and bigger fonts:
image

Don't be shocked but actually most animation software do NOT have status bars. Check Opentoonz, TB Harmony, CACANi, Synfig, Tupitube, After Effects, to name a few. If anything Painting & Design applications are the ones that do have them (Like Krita, Inkscape. GIMP, Photoshop, Illustrator, etc)

I'm saying this because I don't think the status bar should contain too much information, it shouldn't be a place where we put whatever we deem necessary, for the sake of always having it available, since as @J5lx mentions, the more we add to the status bar, the more the min. width will grow, plus it creates duplicate UI for no reason.

I sort of agree, normally status bars should focus on relaying complementary information to the main flow of the program.

To me complementary info that's not easily accessible in Pencil2D can be:

  • Tool descriptions (should change according to selected layer type)
  • Alternative functionality descriptors (for tools like the polyline or the smudge)
  • Relevant tool shortcuts
  • Document size settings, like resolution, aspect ratio or color profile (if applicable),
  • Performance profiling (CPU, GPU, RAM consumption)
  • Pointer coordinates (sometimes these are also paired with per-pixel RGBA information)

This is how MyPaint solves their status bar and it's the same layout both in and out of fullscreen mode
image

This is Krita's
image

So far I like what I see on J5lx proposal, like the tool descriptions and potentially extra shortcuts like for compound tools like the smudge tool (smudge vs blur)

The zoom widget is kind of standard in most painting programs, and while I think a slider makes more sense than a dropdown box, even then having that facility will probably push for having an input box with a presets dropdown when the user can't reach a specific number they want.

Is it the best place to put it though? probably no, but if it's not there, the other "standard" way is to have a viewer panel with the slider or to place it in the tool options when the hand tool is selected. That would honestly take more space than if it was placed on the toolbar IMO.

If we're not using a fullscreen mode however I don't quite understand the purpose of the "current layer" widget I see on the status bar. You should be able to see which layer you have selected on the timeline, though to be fair in practice that's not the case 90% of the time.

For example yesterday while debugging Pencil2D with scribblemaniac, I made a simple mistake of trying to draw while being on the sound layer, and I was wondering why it wouldn't draw. My least obvious response was to look at the timeline, the status bar or anywhere outside the canvas....that's why that sort of info needs to be ON canvas, so you can be notified when you have a narrow field of vision (i.e drawing strokes) and fade out of view once you are notified.

Krita actually does this all the time, but it's more obvious when you enable the canvas-only mode along the fullscreen mode
image

Also to me a true fullscreen mode means a panel-less canvas not the standard maximized mode. I see Krita and MyPaint separate this into two. A fullscreen mode and is put on top of the OS task bar and a Canvas-only mode that hides any unnecessary panels.

So once we enter a canvas-only mode we should not be able to see the status bar in the first place. Hence a full blown screen canvas mode, would require notification overlays. That's what I had in mind when writing the comment in 1151.

Regarding the unsaved / saved, I'll be honest I barely look at the top bar and the status bar for that. If we had the operations widget with the save icon, I would have the disabled / enabled suggestion work there instead (so disabled icon means you don't have modifications pending, and enabled means you are allowed to save).

Perhaps It could work on the status bar but i feel it's an more important piece of visual information than a shortcut which can be missed if the user forgets (or simply doesn't know due to lack of documentation) that they have to look at the status bar for that.

@Jose-Moreno
Copy link
Member

Don't be shocked but actually most animation software do NOT have status bars. Check Opentoonz, TB Harmony, CACANi, Synfig, Tupitube, After Effects, to name a few. If anything Painting & Design applications are the ones that do have them (Like Krita, Inkscape. GIMP, Photoshop, Illustrator, etc)

Sorry I should be more specific regarding this comment. Normally the mentioned animation programs use the concept of a "viewer" or player window which comes with a console widget that has playback controls, FPS counter / FPS rate, zoom / view widgets, so the canvas is part of this viewer widget (more like embedded) and thus this complementary information tends to be closer to the canvas but not necessarily as a part of the timeline, though in a few cases the timeline does hold this info as well.

@J5lx
Copy link
Member Author

J5lx commented Sep 1, 2020

So once we enter a canvas-only mode we should not be able to see the status bar in the first place.

Interesting point. To be honest, I always associated that “canvas-only” mode purely with hiding the dockable windows, and making it possible to hide the status bar separately. I don’t think there’s any particular reason for that, it’s just what comes to me naturally. Actually, I think I tend to consider those features more isolated from each other in general, unlike you – hence why the canvas-only mode doesn’t “imply” the on-canvas display for me the way it does for you. I also considered adding these widgets more as a way to ensure that that information is available to the user (at their disposal), rather than “forcing it into their consciousness” (for lack of a better expression) as you explained in your example – and for that, the status bar seemed adequate to me (plus, it is criminally underutilised as it is now). So that is why I implemented this PR the way I did. You did make some fair points however, so let’s think how to go about that.

The way I understand your comments, you are okay with the tool help and, even though it might not be perfect, the zoom control in the status bar (correct me if I’m wrong). That leaves us with the modification indicator, the layer widget and the timecode display (in David’s PR). For the modification indicator, you suggested integrating it into an operations widget, but since we don’t currently have one we might keep it in the status bar for the time being (with Oliver’s suggestion) – it might not be the best place, but one that users might now from other applications. Now, I suppose the layer name and the timecode – and perhaps also a temporary display of the zoom level when required, like Krita – are things you would like to see on-canvas instead of the status bar, is that right? Should that be shown only in canvas-only mode or when the relevant windows are hidden or always? Plus, we make it so that the canvas-only mode always hides the status bar, regardless of whether it would be shown otherwise, yes? Does that sound good to you or is there something I missed/misunderstood?

@davidlamhauge
Copy link
Contributor

I've just tested this branch, and I like it. Simple as that.
And - that brings me to the question: Should I keep my PR, or should we scrap mine and keep yours?
It seems odd to have two PR's, about the same subject, and it's easy to implement/copy what I have in my PR.

@J5lx
Copy link
Member Author

J5lx commented Sep 2, 2020

TBH, either way is fine by me. Whether we keep the PRs separate, or merge your timecode implementation into this one or even dig up the timecode implementation that was originally part of my changes, I don’t think it makes much of a difference in terms of effort. Whatever you (and perhaps the others) think works best is okay with me.

@Jose-Moreno
Copy link
Member

@J5lx I'm ok with the status bar as it is. We can move the modification indicator behavior to a future operations widget once it's re-implemented using QToolBar as chchwy suggested.

Regarding the timecode I'm sorry I never mentioned otherwise, but In general this should be on the timeline widget, not the status bar; have it closer to the focal area of the animation work which is the canvas.

Here's an ideal breakdown of how the currently discussed features (along with those from 1151 + extra) could be sorted:

Note: It would be worth considering making any status bar element inherit a toggle-able functionality so users can customize it in case it's too crowded

Currently Discussed Features

  • Timecode > Timeline. Reasons above.
  • Current Frame Indicator > Canvas-mode (no panels) only > Use a Canvas overlay
  • Real Time FPS > Canvas-mode (no panels) only > Use a Canvas overlay
  • Layer Selector / Name > Status Bar Ok if full-screen with panels. As a preferred option i'd put it in the operations widgets which is closer to the canvas to improve cognitive search of the feature. Otherwise a canvas overlay would be preferred when there are no panels, but honestly it would be weird to select the layers on a floating panel, so probably it should only be a read-only display name of the current layer
  • Tooltips > Status Bar
  • Shortcuts > Status Bar
  • Zoom slider > Status Bar Ok, Would be more relevant on the canvas as an onscreen canvas widget. For reference look into Animation Paper transform on-screen widgets.
  • Zoom display presets / percentage viewer > Status Bar (basically the referenced Krita display widget)
  • Modification indicator > Status Bar Ok. Would be more relevant on the (yet to be implemented) operations widget.

New Ideas

  • Preset Switcher > Status Bar
  • Resolution Display > Status Bar
  • Memory Cache Fill State > Status Bar (this would have to wait until chchwy's PR is merged)
  • RAM / CPU

Regarding the merge order, this is just my overall impression, but I take this PR more as an enhancement of a collective of features, so I suggest David's PR to be merged first followed by your's, that way you could also finalize improvements over his implementation. This is not only because your seem to cover more ground, but also to clarify the difference in scope and preserve historical continuity of each contribution from the team when possible. My 2 cents.

@davidlamhauge
Copy link
Contributor

I've looked more into it, and I definitely think we should scrap my PR, for at least two reasons:

  1. In your PR there is a statusbar class, so my would need to be refactored later anyways.
  2. As Jose suggests, the timecode maybe should be in the timeline, and then it makes no sense to have a PR that places it in the status bar.
    It's fine with me. I don't think of it as wasted work.

@Jose-Moreno
Copy link
Member

@davidlamhauge Well since i'm not very acquainted with your other PR aside from the result, can't we salvage the functions and calculations created for the formatting? or is it something that is already included in @J5lx PR?

@davidlamhauge
Copy link
Contributor

I'd say it's easier to copy what you can use from my PR, in stead of reviewing my PR, merge it into master, and then refactoring it, moving it to the timeline etc.
I have no intentions of deleting the branch, as long as it's contents is not implemented in some way.

@J5lx J5lx force-pushed the features/statusbar-enhancements branch from e64159d to 1b92831 Compare September 4, 2020 14:27
@J5lx
Copy link
Member Author

J5lx commented Sep 4, 2020

I’ve modified this PR as discussed on Discord. I was originally going to make the status bar contents customisable as well, but I realised that there are actually so few elements left now that it would be kinda overkill. Therefore I decided to hold that part off until we have more stuff on there to justify that feature. Instead, I added the ability to hide the status bar as a whole for those who’d like to save some screen real estate.

@J5lx J5lx force-pushed the features/statusbar-enhancements branch from 036392c to d10113f Compare April 7, 2021 09:39
Copy link
Member

@scribblemaniac scribblemaniac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed this PR and it mostly looks good. I send a PR for a couple minor things and to reduce the minimum width requirements when the toolbar is visible.

There are three outstanding problems:

  1. There is no number/percentage localization. For some languages, the % should be on the other side, or they use a different character, or the use a different decimal separator, etc. Passing hardcoded strings to the combo box is not ideal.
  2. Arrow keys, mouse wheel, etc. do not function as you would expect after a custom number has been entered. The simplest solution I can think of right now would be to allow for one custom entry in the combo box list, make sure that entry is the last custom value used, and that it is placed in the correct position on the list.
  3. The zoom does not update under some circumstances, such as switching to and from a camera layer. This issue is pre-existing, but I am mentioning for completeness.

None of these issues are serious enough to block this from being merged, but I would like to see issues filed for them if this is merged and no issue already exists for each of them.

@J5lx
Copy link
Member Author

J5lx commented Apr 24, 2021

There is no number/percentage localization

I’m going to look into this, so please hold off the merge for now.

Arrow keys, mouse wheel, etc. do not function as you would expect after a custom number has been entered.

I can’t reproduce this. Both arrow keys and mouse wheel work just fine for me even after entering a custom value.

@J5lx
Copy link
Member Author

J5lx commented Apr 25, 2021

I added some localisation to the zoom box. I’m not quite sure on how to handle the position of the percent sign though, so please take a look. @scribblemaniac

@scribblemaniac
Copy link
Member

I added some localisation to the zoom box. I’m not quite sure on how to handle the position of the percent sign though, so please take a look. @scribblemaniac

I looked into this issue before even making the comment. The reason why I did not insist on this being fixed is because there does not seem to actually be any good way to do this. The solution you have come up with is basically as good as I could come up with. We could do something hackish like tr("%1%2).arg(locale.toString(...), locale.percent()), but even with comments that would be quite difficult for our translators to understand. I will have one last look over this PR and merge later this week.

@J5lx J5lx force-pushed the features/statusbar-enhancements branch from 9a59b1a to 9e60eca Compare May 11, 2021 01:25
@chchwy
Copy link
Member

chchwy commented Jul 14, 2021

This PR is pretty much ready to be merged, right? any other concern apart from the percentage sign localization?

@Jose-Moreno
Copy link
Member

This PR is pretty much ready to be merged, right? any other concern apart from the percentage sign localization?

@J5lx What do you think?

@J5lx
Copy link
Member Author

J5lx commented Jul 22, 2021

@Jose-Moreno As far as I’m concerned, this PR has been ready for a while, however @scribblemaniac wanted to look over it one more tiem.

@scribblemaniac scribblemaniac merged commit 57966e7 into pencil2d:master Jul 24, 2021
@scribblemaniac
Copy link
Member

Merged. I will open issues for the remaining problems noted in my review.

@Jose-Moreno
Copy link
Member

@J5lx @scribblemaniac thank you both for the swift answer 🙇

@J5lx J5lx deleted the features/statusbar-enhancements branch July 24, 2021 19:20
@MrStevns MrStevns added this to the v0.6.7 milestone Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement UX Related to the way users interact with the program
Projects
Development

Successfully merging this pull request may close these issues.

6 participants