Page MenuHomePhabricator

efl.ui.factory
Open, TODOPublic

Description

|interface Efl.Ui.Factory
|├ (M) create
|├ (M) release
|├ (E) created
zmike created this task.Jan 8 2019, 11:42 AM
zmike triaged this task as TODO priority.
zmike moved this task from Backlog to Evaluating on the efl: api board.Jan 28 2019, 8:38 AM
woohyun added a comment.EditedJan 28 2019, 11:27 PM

@SanghyeonLee @cedric @felipealmeida

I think Efl.Ui.Factory needs to include only "create", "release", and "created" event.
That is, "model_connect" needs to be included to another interface (or class).

I've heard from @SanghyeonLee that MVVM related things are still in changing, so please check this task together :)

I am not to sure why Efl.Ui.Layout is also an Efl.Ui.Factory by itself. This was done a long time ago with commit 6b9a35d7638a1ef3ffb7cb32ba43daead2f28627 . I am not convinced that it is what we really wanted.

I think we might move the specific call to bind another factory inside a factory to a specific Efl.Ui.GroupFactory or something like that as described in task T7405. Maybe this should actually require an interface so that Efl.Ui.Layout can use it. The logic was for Efl.Ui.Layout to be able to automatically create swallow by using the defined factory. Will have to think about this more and get this fixed in T7405.

zmike added a comment.Jan 30 2019, 6:04 AM

I am not to sure why Efl.Ui.Layout is also an Efl.Ui.Factory by itself. This was done a long time ago with commit 6b9a35d7638a1ef3ffb7cb32ba43daead2f28627 . I am not convinced that it is what we really wanted.

I did rEFL6b9a35d7638a1ef3ffb7cb32ba43daead2f28627 at your request, so I think probably only you know whether this is what we wanted.

Yes, I know and I don't know what I was thinking.

bu5hm4n updated the task description. (Show Details)Feb 22 2019, 1:27 AM

Conceptionally this looks small and just fine, the docs are a bit weird to someone reading it the first time, but that does not block stabilization. @segfaultxavi what do you say ?

@cedric I think i found a problem, Efl.Ui.Widget_Factory, takes either a Efl.Ui.View or Efl.Ui.Widget class, which it uses to create the objects, but the objects need to be Efl.Gfx.Entity, should this be changed ?

The docs are a bit weird to someone reading it the first time, but that does not block stabilization. @segfaultxavi what do you say ?

I say that the docs could certainly be improved (it's in my list), but that does not block stabilization.

zmike added a comment.Jun 12 2019, 9:40 AM

So is this okay or what?

I haven't had time to look in more details to Efl.Ui.ListView and Efl.Ui.GridView that should be using this and would validate the API. According to @bu5hm4n review of those component, I will have to spend some times next week looking at them.

As we discussed in voice call meeting,
Event abstraction on item can be the additional role of factory.
here my question is, if factory can abstract the event callback add,
what object should be passed as the result of this event? item object or widget object with index as event info?

Still event can be propagate to parent widget- listview, gridview - but in that case, event name need to be distinguished as it comes from item, I personally prefer the new prefix "item,"
so the event original is changed as
"clicked" -> "item,clicked"
"selected" -> "item,selected"
"focused" -> "item,focused"

any idea for this?

zmike added a comment.Jun 18 2019, 9:20 AM

As we discussed in voice call meeting,
Event abstraction on item can be the additional role of factory.
here my question is, if factory can abstract the event callback add,
what object should be passed as the result of this event? item object or widget object with index as event info?

If the callback is created for the widget, I would expect the event info should trigger with the newly-created item object.

Still event can be propagate to parent widget- listview, gridview - but in that case, event name need to be distinguished as it comes from item, I personally prefer the new prefix "item,"
so the event original is changed as
"clicked" -> "item,clicked"
"selected" -> "item,selected"
"focused" -> "item,focused"

any idea for this?

Err...so we are talking about the difference in [setting a callback on an item to see when it is clicked] vs [setting a callback on the container widget to see when any item is clicked]? For these cases, I think using item, as a prefix (and emitting that item object in event info) should be fine.

SanghyeonLee added a comment.EditedJul 9 2019, 5:30 AM

@zmike, yes,
and in the case item-emitted callback,
factory provide the abstract way of adding event on the item in mvvm case,
we don't have any prototype for this,
so I might be earlier to be stabilized...

but create, release is stable API as I think...
we could update the item event control after stable this class as I think it didn't breaking any backward compatibility.

SanghyeonLee added a comment.EditedJul 11 2019, 4:43 AM

I think all three MVVM related ticket on stabilized,
T7578
T7579
T7580
would be better to halt stabilization till the MVVM implementation and mono binding are stable.
any idea?

bu5hm4n added a comment.EditedTue, Aug 20, 11:55 PM

@cedric I still see a lot of patches to factory and other things related to that, is it okay if we keep this out of "Evalulating" for now ?

@SanghyeonLee @lauromoura This API seems okay, even if work is happening with factory?

zmike added a comment.Mon, Sep 2, 5:50 AM

Or do we need more functionality?

We should be able to review it next week as work on Collection_View has uncovered a lot of shortcoming.