Differential D407
Use checkboxes for mute in animation editors Authored by Diego Gangl (januz) on Mar 16 2014, 11:13 PM.
Details The current icon for muting an animation in the Graph/NLA editors is a speaker. This is confusing since the toggle doesn't really have anything to do with sound. This patch changes it to checkboxes that can be disabled to mute. 
Updated Screens Dark theme  Default 
Diff Detail Event TimelineComment Actions It's fine with me, the speakers are bit strange. @Joshua Leung (aligorith), any opinion on this? Comment Actions In its current form, I don't think this patch should be accepted. Functionality Wise: One of the reasons why I eventually went with the speaker icons was to take advantage of the "speaker muting" / volume metaphor that many users are already familiar with. (The other being the visual weight issue). Unfortunately, it seems that the meaning behind this is not communicating for some users. Code Wise: Just because you want a checkbox there, you shouldn't hardcode those buttons to point to the checkbox icon, getting rid of the dedicated icon slots reserved for the icons in the animation editors, reducing future customisability. Being forced to invert other usages (not to mention that you've probably missed a great number of these too) of these icons just so that they work correctly for this case is also a big no-no. Comment Actions
The problem with the speaker icons is that they're inherently related to sound. When I first saw these I thought that you could somehow link sound to animation and the speaker allowed you to mute the sound.
My first idea was to replace the icons in the svg, but I thought the speaker icons could be re-used for something else in the future and the checkboxes should be re-used for consitency. Maybe a cleaner solution would be to move the muted speaker icon to the emtpy slot besides the clock (in the animation line), and replace the icons in the svg? No code changes (except icons.h) Comment Actions I made a mockup with @Joshua Leung (aligorith)'s suggestions  What do you guys think about replacing the icons (instead of the code workaround)? Comment Actions I have no trouble with the checkboxes being low-contrast in the initial implementation. This: looks good to me. As @Brecht Van Lommel (brecht) said, the checkboxes have improved readability over the speakers. Comment Actions The good thing about the initial look is that it looked less overwhelming in long lists, but the white inside the checkbox was so transparent it almost looked "disabled". I'll try cranking down the white to be more subtle. Comment Actions We are talking about this? https://dl.dropboxusercontent.com/s/baa9xsu3yq0tlzv/shot_140724_001824.png For me it looks very clear which checkbox is checked. Comment Actions Yeah, I mean users could think it's a disabled checked checkbox, like the ones in the properties panels. Comment Actions
Didn't know that acronym, cool!. What do you think @Joshua Leung (aligorith)? This comment was removed by Diego Gangl (januz). This comment was removed by Diego Gangl (januz). Comment Actions Finally I was able to make a working patch again. Sorry about all the mess. Here's an updated screen  Comment Actions Sorry for not showing up here for so long. I wouldn't really like substituting the speakers here. They are used by plugins still and I could well envision the audio system of blender using them in the future. If the new icons are to be used then I'd suggest to add them on new slots on the svg, add new icon defines for them and use those instead. This comment was removed by Diego Gangl (januz). Comment Actions
Comment Actions Sorry about the version bump on Inkscape. Seems like the new version (0.91) creates a different path property (using features from SVG 2.0). Comment Actions Why aren't the existing checkboxes icons OK? Also agree with @Antonis Ryakiotakis (psy-fi) that the existing icons shouldn't be replaced. Comment Actions The current checkboxes looked kind of small and misaligned with the lock. They also didn't look right in the modifiers. Also I remember I had to do some funky changes because the code in anim_channels_defines.c expected them to be in a certain place (on/off) and the checkboxes in the svg were in the reverse position (off/on). It's in the first diff. The latest diff doesn't replace the icons, only adds the new checks. Comment Actions The patch wasn't yet updated, requesting changes. regarding the icon state being negated, Blender has toggle buttons to handle this see: UI_BTYPE_ICON_TOGGLE_N / UI_BTYPE_ICON_TOGGLE use in draw_setting_widget. If the checkbox icons need to be tweaked for allignment, then we can look into that too. Comment Actions Rebased, but the SVG in master is still using the old inkscape 0.48 so this patch has lots of unwanted changes.
It's not just alignment, the the scale and alpha of the paths was also tweaked.  Comment Actions Rebased The inkscape version in master is now 0.91 too, so this patch doesn't include all the messy changes from the previous one. Comment Actions Abandoning this for now so it doesn't show on everyone's review queue. There's probably a better solution than making more checkbox icons anyways. Comment Actions Hey! What was undecided? The speaker icon should be used for audio-related operations, it doesn't belong in the Graph Editor. In Blender 2.8, Collections use checkboxes for their enabled status. I see no problem in just re-using that checkbox icon, or the checkbox widget itself. |
