Hmm I don't know how I feel about this change.

I prefer the use of icons over checkboxes when they provide more context, which I think they do in this case. "Previous frames" & "Next frames" doesn't say enough to me about what it does. It does conserve a bit more horizontal space though, which is also nice.

Hmm tbh I don’t think the icons provide that much context at all – not in this particular case anyway. Especially when we consider the icon redesign where it’s just a generic lightbulb in both cases. That said, what context is it that you are missing with the checkboxes? If it’s the “visibility” aspect, maybe the labels could be renamed to “Show Previous/Next Frames”?

OTOH, I’d argue the checkboxes do a better job of communicating context through hierarchy. There’s the tool window named “Onion Skins”, gathering all the options related to those. Then inside that there are checkable group boxes to toggle onion skins for previous/next frames. And then inside that in turn are the options to customise those onion skins, which are only effective when the checkboxes are enabled.

Actually, it’s that lack of hierarchy which really bothered me about the old/current design and which motivated me to submit this change in the first place. If you have other ideas to address that, please share them!

No ideas right now, just a comment after trying it out. The checkboxes looks rather small on Mac OS and somewhat out of place. The reason for that might be that there's no native UI component for putting a checkbox on a grouped element, so Qt tries mitigate that.

Here's a quick change with checkboxes inside instead.

Looking at them side by side I still prefer the current but let's hear the others.
As for the hierarchy it creates, I agree it's better. At least in your picture.

Hmm I wonder if there’s a way to address the macOS design issue using stylesheets or something… but as you say, let’s hear the others.

Honestly I'm torn between the two modifications.

The checkbox implementation seems to try to solve the current state of the onion skins where we have the following issues:

1. From user feedback, we've been told it is sometimes unclear how to enable the onion skin feature since the cell layer button to the left does not resemble a button until it is pressed.
2. Additionally it is not obvious at first glance that the feature is enabled / disabled due to the lack of visual cues.
3. When I made the onion skin layout I honestly was wrestling with Qt's UI widgets, but I've always thought that having these sections (Prev. Frame, Next. Frame) stacked like "drawers" might have not been the best choice. If we consider the horizontal nature of the timeline, I sometimes feel it's an additional cognitive obstacle to find out how these options are supposed to work.

Checkbox Implementation

Thinking positively, the check box proposal made by @J5lx currently seems address the first and second issues pointed above. Elements being subordinated visually & greying out every other elements do help provide a visual hint of the current state of the onion skin at a glance.

(New) Icon Implementation

@MrStevns has been working on improving the icons that were left over in PR #1361 He's showed progress screencaps and at a point we had agreed that the icon that could work best to replace the current ones was a lightbulb since it's been canonically used in other animation software to represents the concept of a light table or light box that lits papers from behind so they become see through.

Onion Skins UI / UX redesign

Now considering #1085 this request was made by users on other hubs where they asked for a global toggle to enable disable the onion skins instead of having to disable the previous & next options.

I have frankly needed this too when I'm animating and tried to optimize my workflow for trace backs, although its more of a UX thing, but I thought we could talk about it regarding the UI parts that could help solve the issue.

So with that said I was wondering: What if we had the lightbulb be turned "OFF" with a clear boundary to make it seem like a button (as MrStevns did), as well as having the subordinate items are also greyed out (as J5lx did), this could help showcase that the onion skin status quite well IMO.

If we turn the light bulb "ON" all the other elements would lit up and would help construct the meaning of boolean functionality for the user.

I always thought this general activation button should be the actual lightbulb icon, and in that case we could have the checkboxes for the Prev / Next frames as well. Here's what I'm picturing so we can discuss about it.

Left: Onion Skins panel from March 11th | Right: Onion skins panels with both icons and checkboxes to provide visual hierarchy

If the feature was turned off completely, this is how I think it could look

Let me know what you think 👋

@Jose-Moreno Thank you for that suggestion, That’s really well thought out. I quite like that draft, let’s see what @MrStevns thinks.

As always a very nice and thought-through comment @Jose-Moreno 👏
I like the draft too! the question is whether we can make it look similar using the standard UI components provided by Qt.

You also make a good point that the red/blue light bulb icon serve better as an indication of the onion skin exclusively and we should avoid trying to convey multiple things in one icon. You didn't write that explicitly but I could see it in your draft.

These icons however

feels misplaced now, in that they have little to do with min and max opacity but aside from that I like the draft 😄
We could simply remove them, we already have the text explaining what the icons apply to.

Sorry for the late reply it's been a hectic week.

As always a very nice and thought-through comment @Jose-Moreno 👏 I like the draft too! the question is whether we can make it look similar using the standard UI components provided by Qt.

Sure thing! I had to do a vector mockup since I didn't have much time that day, but whatever Qt allows us to do is fine by me 🙏

You also make a good point that the red/blue light bulb icon serve better as an indication of the onion skin exclusively and we should avoid trying to convey multiple things in one icon. You didn't write that explicitly but I could see it in your draft.

Ah, well I didn't mention it because I wasn't exactly opposed. I was just trying to see how it looked as an alternative and I didn't have time to copy the exact same icon... though I just recalled I could have downloaded them from the PR... 😂

These icons however feels misplaced now, in that they have little to do with min and max opacity but aside from that I like the draft 😄 We could simply remove them, we already have the text explaining what the icons apply to.

I don't have attachment for these icons to be honest, I simply thought It could help to have a visual cue so I put them there because at the time they seemed really fitting to explain the opacity threshold limits, but I won't miss them if we remove them😄

@MrStevns I was thinking about the group box thing again. Would you mind experimenting a little with QGroupBox::title { subcontrol-origin: [margin|border|padding|content]; subcontrol-position: [top|center|bottom] left } on your Mac to see you are able to resolve the issue with the checkboxes looking out of place that way? Or alternatively, maybe something like QGroupBox { margin-top: -x } QGroupBox::title { top: x } or QGroupBox { margin-left: x } (with $x \in \mathbb{N}$)? Or maybe enabling the flat property on the group box could help too.

Thanks for the suggestions, I've tweaked it now and i'm quite satisfied with the result.

In the future we should consider making our own custom widgets rather than modifying individual widgets but for now, since we are not using it elsewhere, we'll keep it in the Onion onion widget.

I also had to tweak the spacing of the widget on Mac OS, because for some reason it gets inconsistent between the first and second group box... seems like a another bug, which might have gone unnoticed until now as I can still reproduce it in in Qt dev branch 😩

Just to prove the point, here's what happens when I set the spacing to 16...

I would have expected the spacing to be seen on each element, however it only happens between the first and second group box.. and then the checkboxes at the bottom. Talk about an infuriating design bug... 😅

Putting that aside:
I have pulled the latest changes into the PR and applied my Mac OS tweaks.
When the redesign PR lands, we could consider making the proposal @Jose-Moreno mentioned.

If you have no further adjustments, feel free to merge the PR.

Thanks a lot for giving it a go! I’m glad you were able to make it work. I do indeed still want to give Jose’s suggestions a shot as well, but I’ll go with your suggestion and keep that separate.