Unchecked instances visually unclear

[BigRoy]

Issue

Not sure where this got introduced, but in the latest version of Pyblish QML unchecked instances are visually hard to read because the checkbox remains a fully solid checked state.

In this image the cameraDefault instance is actually disabled, it's clicked to be deactivated.

The only hint that visualizes it is disabled is the darker greyed out text.

Previously it would also clear the solid contents of the checkbox and keep only the border, which visually made much more sense.

Proposal

Remove the solid checked color when deactivated, like:

---

Could be from this change which was shown here.

[mottosso]

How come it is green? Need reproducible.

[BigRoy]

How come it is green? Need reproducible.

Actually, good question.

import pyblish.api as api

api.deregister_all_paths()
api.deregister_all_plugins()

class CollectorA(api.ContextPlugin):
    order = api.CollectorOrder

    def process(self, context):
        instance = context.create_instance("A")
        instance.data["families"] = ["A"]

class CollectorPass(api.ContextPlugin):
    order = api.CollectorOrder + 0.1

    def process(self, context):
        pass

api.register_plugin(CollectorA)
api.register_plugin(CollectorPass)

The above stays white.

Odd scenario (Likely from backwards compatibility conversion?)

Now on the CollectorPass change the second argument of the process method to instance. It will still run over the context, since it's a ContextPlugin yet it will now start turning the Instance green.

Use case: InstancePlugin in Collector range

We sometimes use a Collector that does some additional collecting on instances of a specific family - in those cases it will always turn green. Example code:

import pyblish.api as api

api.deregister_all_paths()
api.deregister_all_plugins()

class CollectorA(api.ContextPlugin):
    order = api.CollectorOrder

    def process(self, context):
        instance = context.create_instance("A")
        instance.data["families"] = ["A"]

class CollectorA2(api.InstancePlugin):
    order = api.CollectorOrder + 0.1

    def process(self, instance):
        instance.data["additionalData"] = 1

api.register_plugin(CollectorA)
api.register_plugin(CollectorA2)

---

So that's two cases.

[BigRoy]

@tokejepsen @mottosso what do you believe is the intended result in the above cases?

[mottosso]

Now on the CollectorPass change the second argument of the process method to instance.

Yes, don't do this, it's from the days of "dependency injection". ContextPlugin should only ever have context, and InstancePlugin should only ever have instance.

Sorry can't look closer at the moment but will try in the coming days.

[BigRoy]

Yes, don't do this, it's from the days of "dependency injection". ContextPlugin should only ever have context, and InstancePlugin should only ever have instance.

Yup, was expecting that to be related. Then the only real issue here is the latter use case I suppose.

[BigRoy]

Can anyone else confirm this issue?

I'm expecting the use case I showed above (Use case: InstancePlugin in Collector range) to be a valid use case where after collection the instance should not appear green when you uncheck the instance - it should look unchecked.

The fix I set up before works for me, there an unchecked instance will never be highlighted green, orange or red. (Aside from its border keeping the color though!)

For me this is a good fix.

[davidlatwe]

I can confirm this, just ran through the example use case above.

Also, can confirm this issue with my previous publish experiences on my plugins and instances, but didn't think too much at that time.

PR #284 seems to improve this well, that works for me, too.