Page MenuHome

Patch Adds Responsive Eye Icon to Nodes
Needs ReviewPublic

Authored by Chris Stones (chrisstones) on Nov 30 2015, 5:30 AM.

Details

Summary

Spent days not knowing I had to click the "orb" until I finally got the answer in irc.
So, I replaced the orb on the node with an eye icon.

Diff Detail

Event Timeline

Chris Stones (chrisstones) retitled this revision from to Patch Adds Responsive Eye Icon to Nodes.
Chris Stones (chrisstones) updated this object.
Chris Stones (chrisstones) updated this object.
Chris Stones (chrisstones) updated this object.

Makes sense, the material icon might work for Cycles but not really for compo. This icon works everywhere.

@Julian Eisel (Severin), @Jonathan Williamson (carter2422) What do you guys think?

Change looks logical, and is more descriptive than the current one.

This revision is now accepted and ready to land.Dec 8 2015, 11:31 AM
Julian Eisel (Severin) requested changes to this revision.EditedDec 8 2015, 10:26 PM
Julian Eisel (Severin) edited edge metadata.

We briefly talked on IRC some days ago, writing down my thoughts here:

UI-wise:
I agree that the current icon is really not fitting here, so +1 for searching for a better alternative. I'm not sure if the eye icon is the best alternative though, I would expect this to be used for muting or hiding the node. Maybe a magnifying glass or render icon? @Jonathan Williamson (carter2422), @venomgfx, @Paweł Łyczkowski (plyczkowski), or anyone else - opinion?

Code-wise
In IRC, I said that it's possible to let the interface code automatically handle the icon state (enabled or disabled), but I realized this currently only works if the interface code also handles setting the RNA property value or the bit flag controlled by the button. This however, is currently handled in NODE_OT_preview_toggle (for various reasons), so let's keep things simple and just set the icon manually :D

To avoid code duplication I'd rather do this:

const int icon = (node->flag & NODE_PREVIEW) ? ICON_RESTRICT_VIEW_OFF : ICON_RESTRICT_VIEW_ON;
...
but = uiDefIconBut(..., icon, ...)

(NULL-checking previews also shouldn't be needed)

This revision now requires changes to proceed.Dec 8 2015, 10:26 PM

We briefly talked on IRC some days ago, writing down my thoughts here:

UI-wise:
I agree that the current icon is really not fitting here, so +1 for searching for a better alternative. I'm not sure if the eye icon is the best alternative though, I would expect this to be used for muting or hiding the node. Maybe a magnifying glass or render icon? @Jonathan Williamson (carter2422), @venomgfx, @Paweł Łyczkowski (plyczkowski), or anyone else - opinion?

Well, I think the verb here is "to see" which means eye to me. I didn't think the original icon was a button. When the compositor loads now. The open eye with the preview thumbnail and the added nodes with the closed eye icon solve my original problem of not knowing why I couldn't 'see' the previews. So the button def should have 2 states.

Also, I thought the equiv. of hiding a node was using that triangle to minify it.

Chris Stones (chrisstones) edited edge metadata.

Updated the diff with suggested changes.

I'm not familiar with this ticket system. Did I have to change the state of the issue after I made the update?

No, all fine :)

This is a good suggestion. The material orb really doesn't make any sense here.

I'd vote for the [DISCLOSURE_TRI_RIGHT](http://cl.ly/e5t1 icon, though, as it's more indicative of *show more*, or *show less*. It's also used elsewhere in the UI for the same purpose (such as UI list filtering).

The eye icon, while nice, is too close to *hide* in my opinion, particularly since this is exactly what its' used for elsewhere (such as the outliner).

If push comes to shove, I'm also fine with the eye icon, though :) It's better than the material orb.

I'm not sure about having two triangles in the node title. New users would click the wrong triangle and confuse the two a lot.

The manifying glasses don't have a semi-transparent version that looks "off", so I say +1 for RESTRICT_RENDER_ON/OFF

DISCLOSURE_TRI_RIGHT is in fact not a triangle it's the little +/- icon (the one used in list widgets like for vertex groups to show/hide filter options).

My bad :P, those end with _VEC. I still think the render icons would be more descriptive though.

related note/reminder regarding node icons: custom py nodes dont draw them at all atm. (see D1578: fix Custom Python Nodes dont draw their 'bl_icon', patch included)

Having little eye on each node is a little creepy - an eye is a visually intense icon (since we are wired to find faces), especially in black and white. Also the header is not that logical place for it, since this does not hide/turn off the whole node, but just the preview inside it.

So, I personally would suggest the preview on/off option for a node in the Properties sidebar - it's a feature for advanced users, so does not have to be readily available right on the node IMO.

Eye icon is used for mute already in too many places, think its confusing to use eye here.

Really not so keen on making "advanced user" unless they are really corner-cases... (feedback from other users welcome, but my impression is this isn't such an unusual thing to change)

We could just make this icon more obviously a button: