effects: Add create_property_widget signal to allow custom property widgets in
ClosedPublic

Authored by suhas2go on Jul 11 2017, 5:25 PM.

Diff Detail

Repository
rPTV Pitivi
suhas2go created this revision.Jul 11 2017, 5:25 PM
aleb added inline comments.Jul 11 2017, 5:51 PM
pitivi/effects.py
577

Aborts ...

pitivi/utils/custom_effect_widgets.py
32

Both callback methods should have the _cb suffix.

48

prefer to use quotes instead of apostrophes, for consistency

pitivi/utils/widgets.py
872–873

Nice

1042

make_property_widget

suhas2go marked 4 inline comments as done.Jul 11 2017, 6:27 PM
aleb requested changes to this revision.Jul 15 2017, 9:58 PM

only cosmetic

pitivi/effects.py
600

Must start with a verb. Maybe:

"""Creates a widget if the `create_property_widget` handlers did not."""
This revision now requires changes to proceed.Jul 15 2017, 9:58 PM
suhas2go marked an inline comment as done.Aug 8 2017, 11:23 PM
Mathieu_Du requested changes to this revision.Aug 10 2017, 1:28 AM

Other than my comments, looking nearly ready to merge, well done :)

pitivi/effects.py
621

See my comment below, you shouldn't need to pass self here

pitivi/utils/widgets.py
872–873

Well that's one way to do it, but it really isn't good practice to emit a signal from a different class, I would much prefer if we defined and emitted our own signal here, and relayed it in the effect manager :)

This revision now requires changes to proceed.Aug 10 2017, 1:28 AM
suhas2go updated this revision to Diff 6265.Aug 15 2017, 12:27 AM

Don't pass effect_prop_manager getEffectConfigurationUI

suhas2go marked 2 inline comments as done.Aug 15 2017, 12:37 AM
Mathieu_Du requested changes to this revision.Aug 15 2017, 1:27 AM
Mathieu_Du added inline comments.
pitivi/utils/widgets.py
872–873

Nearly there, for the sake of encapsulation I'd rather you pass effect_prop_manager.create_property_widget as a parameter to that function, think you can do that?

This revision now requires changes to proceed.Aug 15 2017, 1:27 AM
suhas2go marked an inline comment as done.Aug 15 2017, 7:45 PM
Mathieu_Du accepted this revision.Aug 15 2017, 7:47 PM
This revision is now accepted and ready to land.Aug 15 2017, 7:47 PM
Closed by commit rPTV495359ba1574: effects: Allow custom property widgets when auto-generating UI of effects (authored by Suhas Nayak <suhas2go@gmail.com>, committed by aleb). · Explain WhySep 5 2017, 10:55 PM
This revision was automatically updated to reflect the committed changes.