Page MenuHomePhabricator

efl-mono: Add Model manual implementation to C# and MVVM factories
ClosedPublic

Authored by lauromoura on Feb 28 2019, 5:24 PM.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
felipealmeida created this revision.Feb 28 2019, 5:24 PM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

felipealmeida requested review of this revision.Feb 28 2019, 5:24 PM

Some initial comments

src/Makefile_Ecore.am
56 ↗(On Diff #19853)

Are these required or would be under some if HAVE_CSHARP guard?

src/lib/ecore/meson.build
81 ↗(On Diff #19853)

Are these required or could be under a check like if get_option('bindings').contains('mono')?

src/tests/efl_mono/Model.cs
25

Missing some checks here?

39

Use Test.Assert(condition) instead of Console.WriteLine.

49

Remove?

50

Check something after to confirm the property was bound?

src/tests/efl_mono/Parts.cs
23 ↗(On Diff #19853)

Assert something later?

31 ↗(On Diff #19853)

Dynamic get only?

is this patch ready to go?
please comment more detail about this patch in commit-message,
and rebase on latest master.

can you update status of this diff?

lauromoura commandeered this revision.Jun 28 2019, 3:10 PM
lauromoura edited reviewers, added: felipealmeida; removed: lauromoura.
lauromoura updated this revision to Diff 23056.Jun 28 2019, 3:18 PM

Another round with some fixes.

Model tests working

lauromoura updated this revision to Diff 23223.Jul 10 2019, 3:16 PM

Updating after big changes.

  • Rebase
  • MVVM C# classes working
  • Added some docs
  • Improved testing

One change this version adds is that the Efl.Mono_Model_Internal class was
moved to be built into the custom exports library instead of ecore, as it is
a binding-specific class and this library is already imported by the binding.

lauromoura edited the summary of this revision. (Show Details)Jul 10 2019, 3:19 PM
lauromoura edited the test plan for this revision. (Show Details)
lauromoura added a reviewer: segfaultxavi.
lauromoura edited projects, added Restricted Project, efl: language bindings; removed DO NOT MERGE, efl.

what else do we need for this?
if it is ready for merge, please let us know.

SanghyeonLee added a comment.EditedJul 18 2019, 4:39 AM

Test Note,
as I tested in latest version.

var item = new Efl.Ui.ListDefaultItem(ifac);
item.IconPart.SetContent(null);

I can find the part of item IconPart, ExtraPart, TextPart properly with vs IDE,
but,

var ifac = new Efl.Ui.ItemFactory<Efl.Ui.ListDefaultItem>(null);
 ifac. ?

I cannot find any part member of item, IconPart(), ExtraPart(), TextPart().

@felipealmeida is there another way to do this?
I see that using propertyBoundEvt in the test, but can't understand how exactly part is exposed in the event.

lauromoura planned changes to this revision.Jul 22 2019, 8:26 PM

Test Note,
as I tested in latest version.

var item = new Efl.Ui.ListDefaultItem(ifac);
item.IconPart.SetContent(null);

I can find the part of item IconPart, ExtraPart, TextPart properly with vs IDE,
but,

var ifac = new Efl.Ui.ItemFactory<Efl.Ui.ListDefaultItem>(null);
 ifac. ?

I cannot find any part member of item, IconPart(), ExtraPart(), TextPart().

@felipealmeida is there another way to do this?
I see that using propertyBoundEvt in the test, but can't understand how exactly part is exposed in the event.

Looks like the patch is missing adding the extension methods for the parts too (currently it is adding only for the properties). Tomorrow I'll add it alongside some tests and will update the diff.

The idea is to be able to bind properties from the Part to properties of the Model through the Factory, right? Do factories already support binding properties of parts of their item class? (@cedric?)

Assuming that works, the C# syntax for binding parts properties could be something in the lines of:

var factory = new Efl.Ui.ItemFactory<Efl.Ui.ListDefaultItem>();
factory.TextPart().Bind("somePartProperty", "someModelProperty");

(Note: This requires the parts to be declared in Eo files as we would need to generate extension methods for them to we can extend ItemFactory.)

The idea is to be able to bind properties from the Part to properties of the Model through the Factory, right? Do factories already support binding properties of parts of their item class? (@cedric?)

We haven't moved properties to be Part yet.

Assuming that works, the C# syntax for binding parts properties could be something in the lines of:

var factory = new Efl.Ui.ItemFactory<Efl.Ui.ListDefaultItem>();
factory.TextPart().Bind("somePartProperty", "someModelProperty");

(Note: This requires the parts to be declared in Eo files as we would need to generate extension methods for them to we can extend ItemFactory.)

Hum, this is very unlikely as the Part will be defined on the Item class not on the Factory that can build any type of Item. Also a lot of the Part will be implicit from either Layout part or Property part. Not to sure how we could improve that situation.

The idea is to be able to bind properties from the Part to properties of the Model through the Factory, right? Do factories already support binding properties of parts of their item class? (@cedric?)

We haven't moved properties to be Part yet.

Assuming that works, the C# syntax for binding parts properties could be something in the lines of:

var factory = new Efl.Ui.ItemFactory<Efl.Ui.ListDefaultItem>();
factory.TextPart().Bind("somePartProperty", "someModelProperty");

(Note: This requires the parts to be declared in Eo files as we would need to generate extension methods for them to we can extend ItemFactory.)

Hum, this is very unlikely as the Part will be defined on the Item class not on the Factory that can build any type of Item. Also a lot of the Part will be implicit from either Layout part or Property part. Not to sure how we could improve that situation.

Given that there are these open points regarding parts, we could go ahead with this patch (which enable binding item_class properties to model properties) and create a new task specific for parts. What do you think @SanghyeonLee ?

The idea is to be able to bind properties from the Part to properties of the Model through the Factory, right? Do factories already support binding properties of parts of their item class? (@cedric?)

We haven't moved properties to be Part yet.

Assuming that works, the C# syntax for binding parts properties could be something in the lines of:

var factory = new Efl.Ui.ItemFactory<Efl.Ui.ListDefaultItem>();
factory.TextPart().Bind("somePartProperty", "someModelProperty");

(Note: This requires the parts to be declared in Eo files as we would need to generate extension methods for them to we can extend ItemFactory.)

@lauromoura that is what we expected.

Hum, this is very unlikely as the Part will be defined on the Item class not on the Factory that can build any type of Item. Also a lot of the Part will be implicit from either Layout part or Property part. Not to sure how we could improve that situation.

@cedric that is true, but if we not using delegate, easy way of export part is redirecting it in factory..
I don't know how exactly we will expose this item's Part property to factory's method,
but if it possible, we could group them in some structure, or give some prefix can be considerable.

Given that there are these open points regarding parts, we could go ahead with this patch (which enable binding item_class properties to model properties) and create a new task specific for parts. What do you think @SanghyeonLee ?

sure. I think we delayed this patch quite a lot,
and the diff will be grow up every seconds, so I agreed that first push this patch and build new task about part on top of it.

I'm rebasing ( void_ptr broke it) and updating with the marshalling of Eina.ValueType in the internal model.

Rebase and some small updates:

  • Beta guards
  • Proper storage of einar_value_types in model internal

Updating after big changes.

  • Rebase
  • MVVM C# classes working
  • Added some docs
  • Improved testing

    One change this version adds is that the Efl.Mono_Model_Internal class was moved to be built into the custom exports library instead of ecore, as it is a binding-specific class and this library is already imported by the binding.

As the custom exports library changed location with this patch, a clean build may be need. (Actually cleaning the build directory, as ninja clean alone may leave an old customexports.so around).

cedric accepted this revision.Fri, Aug 2, 1:41 PM
This revision is now accepted and ready to land.Fri, Aug 2, 1:41 PM
This revision was automatically updated to reflect the committed changes.