Page MenuHomePhabricator

efl_ui_stack: remove!
ClosedPublic

Authored by bu5hm4n on May 19 2019, 4:00 AM.

Details

Summary

this can now be done with active_view. This is done in order to reduce
the LOC in elementary that basically do the same.

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.
bu5hm4n created this revision.May 19 2019, 4:00 AM
bu5hm4n requested review of this revision.May 19 2019, 4:00 AM
bu5hm4n added a reviewer: woohyun.

Noo... i just found a bug in this widget. :P

@singh.amitesh That makes you a perfect tester for this new widget that should replace it. Can you test it / check it out ? :)

bu5hm4n updated this revision to Diff 22443.May 25 2019, 4:06 AM
bu5hm4n edited the summary of this revision. (Show Details)

introduce a new class to do pop / push

I thought that the util class would be either

  1. a regular class inherits from Efl.Ui.Active_View.Container
    • it sets Efl.Ui.Active_View.View_Manager_Stack in constructor or finalize?
    • it makes view_manager_set disabled (e.g. it overrides view_manager_set and makes other view manager cannot be set)

Or

  1. a regular class attaches Efl.Ui.Active_View.Container as a composite object
    • it sets Efl.Ui.Active_View.View_Manager_Stack in constructor or finalize?
    • it makes view_manager_set disabled (I am not sure how to do it)

What do you think about above?

I don't think it makes sense to create a class called Util that is creatable, and has additionally pop & push, where is there the difference to just having pop & push on the normal object? It makes no sense to restrict the usage of push and pop to something that only can have the stack View_Manager. Why shoudn't someone just use the Scroll View_Manager instead of the stack? It could looks nice and serve the purpose.

I can create here another util function that just returns a ready to use Active_View with a setted stack, nothing against that. But there is no need for another object.

@bu5hm4n

I see. I left comment incorrectly about util class. Please forget about what I talked about util class.

I think that utility class which provides push/pop methods is not a better option than regular class for stack usecase.
Because, as you know, using utility class is not OOP approach so it is not easier than using push/pop from a regular class.
e.g.

  1. Support push/pop by utility class in C#
Efl.Ui.ActiveView.Container container = new Efl.Ui.ActiveView.Container(...);
Efl.Ui.ActiveView.Util.Push(container, content);
  1. Support push/pop by regular class in C#
//Let's assume Efl.Ui.ActiveView.Stack inherits from Efl.Ui.ActiveView.Container
Efl.Ui.ActiveView.Stack stack = new Efl.Ui.ActiveView.Stack(...);
stack.Push(content);

IMO, the second regular class usecase is easier to use for stack usecase not only it adopts general OOP approach but also we can get help from tools like intellisense in Visual Studio.
Basically, I like the class Efl.Ui.ActiveView.Container and its concept. And the reason why I think it is good because we can extend the class for specific usecases.

In the beginning, I thought that the Efl.Ui.ActiveView.Stack's ViewManager is set in constructor and should not be changed.
But now I think that you are right about the ViewManager also can be changed by developer. But I still think the default ViewManager should be set in constructor in Efl.Ui.ActiveView.Stack for convenience.
Because most developers just want to use the default transition. If they want to change the transition, then they can modify it by set another ViewManger as you commented.

If the new Efl.Ui.ActiveView.Stack class inheriting Efl.Ui.ActiveView.Container is beyond this patch, then please you do not need to consider it. I will request a new patch for it later.

Okay, lets say we overwork this one. Can you accept the previous ones, so they can land ? :)
This revision here will stay, and we overwork the approach here.

I am not in favor of a new class inheriting from active_view, just with 2 more APIs push and pop. Push and pop do look general useful to me, and belong IMO either on the Efl.Ui.Active_View.Container class, or like here in a util class.
I think its a bad idea to add a new class because:

  1. There is no real difference in the functionallity the class provides, there are just two more util functions that do not change anything in terms of the internal state.
  2. A function here in this util class like "stack_gen" could just return a Active_View.Container with VIEW_MANAGER_STACK_CLASS as view_manager. Then we would not need another type/class in our tree.
  3. A way more performant and IMO better way for all this can be implemented when its in Active_View.Container.

The Push and Pop behavior can be already implemented by setting the gravity property to index, setting the active_index to 0, and then doing "Push" with pack_begin and "Pop" with unpack_at(0) (all this is documented in the new docs).

If there are animation issues with the above approach, I would rather fix them than adding new classes (inherited or utilities), because it looks like we have almost everything we need already.

@segfaultxavi

As you mentioned, we have animation issues on pop with only using gravity. Because the unpacked view cannot be in the pop animation.

I am afraid it is difficult to support pop animation without adding new methods/properties or supporting new classes.

@segfaultxavi

I thought more about your suggestion about fix.

And I realized that it would be fine if the unpacked view is waiting until transition is finished if the the unpacked view is active view (active_index).

I will test it!! Thanks :)

@segfaultxavi

I tested it and I found a problem.
As you know, unpack does not delete the content so the content should be deleted by developer manually.
Therefore, it becomes the same as developer registers callback for transition finish and delete the content manually ;(

To support push/pop, I write specification for push and pop.
(The names, push and pop, can be changed)

push

  • sets active index to the pushed view (packed view)
  • animation can be controlled to be displayed or not (This can be controlled by "animation_enabled" of View_Manager)

pop

  • sets active index from popped view (unpacked view) to another view
  • can control to remove the popped view (unpacked view) after pop is finished
  • The number of popped view can be more than one (i.e. pop_to() pops multiple views)
  • animation can be controlled to be displayed or not (This can be controlled by "animation_enabled" of View_Manager)

@bu5hm4n @segfaultxavi
How about we add active_view_pack and active_view_unpack methods to Active_View.Container?
If so, active_view_gravity is removed. (we only use content gravity policy)

active_view_pack {
   params {
      view : Efl.Gfx.Entity;
   }
}
active_view_unpack {
   return: Efl.Gfx.Entity;
}
@property active_view_unpack_autodel {
   [[This flag can be used to delete view unpacked by active_view_unpack.
     If the autodel is true, then active_view_unpack deletes unpacked view after transition is finished.]]
   values {
      autodel: bool;
   }
}

If we support above methods, then the C# API would be PackActiveView() and UnpackActiveView(). They look good to me :)
We can add some more methods for convenience.

active_view_pack_at {
   params {
      view : Efl.Gfx.Entity;
      index : int;
   }
}
active_view_unpack_at {
   params {
      index : int;
   }
   return: Efl.Gfx.Entity;
}
active_view_unpack_to {
   [[Unpack views from the current active index view to the given index view]]
   params {
      index : int;
   }
   return: Efl.Gfx.Entity;
}

I think we can decide if active_view_pack packs the given view to the beginning or to the end. (also active_view_unpack)
If we synchronize with other containers such as box, then we need to pack it to the end.

What do you think about adding the above methods to Active_View.Container?

bu5hm4n added a comment.EditedJun 5 2019, 4:57 AM

What is active_view_pack / active_view_unpack doing ?

What is active_view_pack / active_view_unpack doing ?

active_view_pack packs an active view so the packed content becomes an active view (active index's view).
active_view_unpack unpacks the active view so the active view (active index's view) is unpacked.

pack and unpack simply packs and unpacks the content but active_view_pack and active_view_unpack packs and unpacks the active view (active index's view).

I have thought about APIs for push/pop and I think that the following 3 methods are sufficient for our push/pop usecase.

active_view_pack {
   [[Adds a new view to this widget and sets the new view to be the active view.

     The new view is added to the beginning of this widget.
   ]]
   params {
      view : Efl.Gfx.Entity; [[View to add and set to be the active view.]]
   }
   return : bool; [[$false if $view could not be packed.]]
}
active_view_unpack {
   [[Removes the active view from this widget.

     The active index is not changed.
     e.g. The active index is 0, then the new active index is still 0.
     If the active index is removed, then it is changed.
     e.g. There are 2 views and active index is 1, then index 1 is removed so the new active index is 0.
   ]]
   return : bool; [[$false if there is no view in this widget or the active view couldn't be removed.]]
}
@property active_view_unpack_autodel {
   [[This flag can be used to delete view unpacked by active_view_unpack.
     If the autodel is true, then active_view_unpack deletes unpacked view after transition is finished.
   ]]
   values {
      autodel: bool; [[ $true if $active_view_unpack deletes the active view automatically.]]
   }
}

What do you think about the above?

For the usecase pop multiple views, we can do efl_pack_unpack multiple views and do efl_ui_active_view_unpack().

//we have 0, 1, 2, 3 index views and active view index is 0 and want to delete 0, 1, 2 index views
efl_pack_unpack(index 1's view);
efl_pack_unpack(index 2's view);
efl_ui_active_view_unpack();

That works for me, I will implement those APIs later today and test them and look over them at the weekend. We then can merge it on Monday. Thank you for putting time into it :)

To be clear: I will implement those APIs on the efl_ui_active_view_container object. I still think it should not be a separated object. (But I will also provide helper methods in until to get a stack like object)

bu5hm4n updated this revision to Diff 22672.Jun 7 2019, 10:04 PM

implement active_unpack / active_pack

@Jaehyun_Cho I have implemented this now. I basically took your proposal just with three different things :

  • There is no method for autodeleting something, it feels weird to me to have that in a property, having it in a parameter sounds more sensible, since the usecase "I want to delete the content" is IMO not really bound to the widget, rather to a special situation, so it might be different from time to time.
  • The functions do not return true / false anymore. Reason for this is: The API is async, it triggers a animation, that might be immidiatly done, but it also might take a few frames to finish, that means, telling the user *yeah everything worked* while not have the animation / deletion finished, is not something i would do
  • The unpack method now returns an promise. The promise will be resolved once the animation is done, carrying either the Efl.Gfx.Entity (if autodel is false), or simply empty when the autodel is true. In this way the user has a easy way to find out when the operation is done. Without the need to register to the transition,done event, and unregister from it again. (I will add cedric as an reviewer, to verify that i did not do any mistakes here ... :))

@bu5hm4n

Thank you for the implementation! :)
I haven't used future on C# bindings so I think I need to check it on C# as well.

I have been trying to test the C# stuff myself, it seems that it does not work, since Object is not understood by the eina_value implementation of C#, we can say we keep it for now as a promise, the initial proposal did not address that feature anyways, so the main-thing that was requested is there i guess. Right now a user can still get the correct point of time, when he is listens to the next transition,end event ... :)

@bu5hm4n
Basically, I agree with you. I think that the returned future is good.
However, I am concerning if it is available to use EINA_VALUE_TYPE_OBJECT in EFL C# before we release EFL C# bindings publicly.
If the future is returned by active_view_unpack(), then developers want to know what is the eina_value in their eina_future_then callback. (now the Eina_Value argument cannot be used as an object in C#)

@felipealmeida @vitor.sousa
For now, EINA_VALUE_TYPE_OBJECT is not supported in EFL C#. (not supported in eina_value.cs in manual bindings)
What do you think about supporting EINA_VALUE_TYPE_OBJECT in EFL C#? (supporting in eina_value.cs in manual bindings)

I think we should definitely support the object type in the Rina value, mvvm also uses this kind of stuff afair. If it turns out that we don't need it, then I can still update this patch :)

Jaehyun_Cho accepted this revision.Jun 12 2019, 5:33 AM

@bu5hm4n
I see~ You are right we have time to change before release (although it might not be that long! :( ) Thank you :)
Just a rebase is needed to push this~

Do we need to wait for @cedric 's comments on the usage of future/promise in this patch?

This revision is now accepted and ready to land.Jun 12 2019, 5:33 AM

@cedric can you say something about the promise usage ?

segfaultxavi requested changes to this revision.Jun 13 2019, 3:14 AM

I also think adding push & pop to the container makes sense.
I do not like the names active_view_pack/unpack though. They can be confused with the normal pack/unpack from the Efl.Pack_Linear interface.
I think the names push & pop are perfectly understood by everybody, why not use active_view_push/pop? It's less confusing.

I like the Eina_Future, but I think we are unnecessarily complicating things. Why don't we just return the view immediately from pop (just like unpack does), but we keep an extra reference until the animation is finished?
As you have found, support for Eina Futures and Values in C# is still incomplete.

The gravity property needs to be removed if we use these new two methods.

Needs rebasing.

src/lib/elementary/efl_ui_active_view_container.eo
11

Please add this text after line 20:

Adding views through the @Efl.Pack_Linear methods does not change the currently active view (@.active_index). If you want to add a view and immediately activate it you can use @.active_view_push for convenience.
51–52

This property needs to be removed.

60
Packs a new view at the beginning and sets the @active_index to 0.

This is the same behavior as a push operation on a stack.

An animation might be triggered to make the new active view come into position.
69
Removes the active view from the widget.

The views behind it naturally flow down so the next one becomes the active view. This is the same behavior as a pop operation on a stack.
When combined with @.active_view_push you don't have to worry about @.active_index since only the first view is manipulated.

An animation might be triggered to make the new active view come into position and the old one disappear.

The removed view can be returned to the caller or deleted (depending on $delete_on_transition_end).
77

I prefer delete_on_transition_end.

79

Missing doc for the return value, but see my comment regarding Futures first.

This revision now requires changes to proceed.Jun 13 2019, 3:14 AM

After talking with @bu5hm4n I now understand we cannot keep an extra ref to the view while animating, because the user can do all sorts of strange things with it.

But still, I would like to avoid using Futures. @Jaehyun_Cho, would it be an option for you if active_view_pop did NOT return the popped view, so it is always deleted after animation?
I think this is by far the most usual case, and it makes sense to make it simple. If any user wants to push and pop views and still use them after popping, he can do it manually (with pack/unpack, active_index, and listening to the transition,end event.

@Jaehyun_Cho Can you answer if you are happy with pop / push ?

@segfaultxavi in all honesty, not returning the promise is not an option here, if we require that in future we need a new API, which basically does the same. It is perfectly possible right now to do that, bindings do lack this feature, but they should add that.

@bu5hm4n I'm not arguing against using a Promise, I'm just trying to see if there are alternatives :)

So, if we DO return a promise containing the popped view, and the user does not care about this view (I think this would be the most usual case), what does the user need to do? Does he need to attach a callback to the promise just to unref the view? What happens if he just ignores the returned promise?

@segfaultxavi this depends on the parameter, when delete_on_transition_end is true, then the user does not need to care about the value of the promise, only the point of time. When the user passes false as argument, then the user *needs* to handle the value of the promise. However, you have to note here, that when the user does not *want* to take care of that returned value, the widget that got unpackt will just float arround on the UI, which is probebly not what a user wants ... :)

@Jaehyun_Cho Can you answer if you are happy with pop / push ?

@bu5hm4n @segfaultxavi
There are 2 things why I suggested active_view_pack and active_view_unpack instead of push/pop.

  1. To synchronize EFL container's interface method names
    • Using the term "pack" and "unpack" like other container interface method names.
    • active_view_pack and active_view_unpack works like pack/unpack and it also changes active view (active index).
  1. active view container can be non-stack container
    • Stack container does not require "active index" because top view is always the active view. So push() packs a new view to the end of the container and pop() unpacks the end of the container.
    • If push/pop is called with non-stack usecase, push/pop may not suit for the usecase. (e.g. there are 5 pages and active index is 2.)

I totally understand that push/pop is more intuitive and we can simply imagine what they do.
But my point is that active_view_pack and active_view_unpack are not the same concept with push and pop because active view container can be non-stack container.
We can simply rename the active_view_pack/active_view_unpack to push/pop. But I am not sure if the functionalities are the same with push/pop which developers may expect.

@bu5hm4n
What do you think? Do you think push/pop is better name than active_view_pack/active_view_unpack?

@segfaultxavi this depends on the parameter, when delete_on_transition_end is true, then the user does not need to care about the value of the promise [...] When the user passes false as argument, then the user *needs* to handle the value of the promise.

OK, understood. I am OK with using an Eina_Future.

  • active_view_pack and active_view_unpack works like pack/unpack and it also changes active view (active index).

There you have it. active_view_pack/unpack work like standard pack/unpack BUT do one additional thing. Therefore, if we use the same name it will be misleading.
Think about a new user, who knows nothing about the Pack interface. He just wants to create a Scrolled Pager. His IDE autocompletes his object and he sees two options: pack and active_view_pack. He will not know which one to use unless he reads the docs.

push/pop is much more intuitive, and I do not understand your example (there are 5 pages and active index is 2.). We modified the active_view_pack docs to make it clear that it always adds to the beginning, not to active_index.
Inserting in the middle can still be accomplished with pack_at, but I do not think it is so common that it needs a special method.

Sooooooo.... I still like push/pop better than pack/unpack, and I accept pop returning an Eina_Future (with a parameter to automatically destroy the view).

@Jaehyun_Cho i agree with Xavi here. I will do the changes tomorrow.

@segfaultxavi @bu5hm4n
Thank you for the opinion :) It helped me to understand new developers better~ :)

I just want to make one thing clear about pop().

About push(), push() always adds a new view in the beginning.
About pop(), 1. does pop() always removes the beginning view? Or 2. does pop() removes the active view (active index's view)?

For now, active_view_unpack() works second way.

What do you think how pop() should work? first way? or second way?

@Jaehyun_Cho i would actaully say it is fine like it is implemented now. active_index is always 0 after the usage of this API, as you would then use it stack wise ... :)

Hmmmm... @Jaehyun_Cho raises a good point. It is weird that push always packs to index 0 whereas pop unpacks from active_index. We need to use the same approach on both methods.

As long as they are consistent, I think both approaches are OK.
Being able to push and pop from active_index is probably more flexible than always doing to to index 0. If the user only wants to use a Stack, he does not need to worry about active_index (it will always be 0), but if he wants, he can push/pop from the middle.
This is a very strange use case, though, so I will leave the decision to @bu5hm4n, since he's the one implementing it :)

bu5hm4n updated this revision to Diff 22794.Jun 18 2019, 3:32 AM
bu5hm4n edited the summary of this revision. (Show Details)

apply latested spoked about changed

Sooo the approach implemented here now uses active_index. so push and pop inserts new widgets at $.active_index. However, if you never touch active_index by hand, and only uses push / pop, then active_index will always stay 0 :)

Some of my comments regarding docs have not been addressed yet, so I am not approving this!
(Remember to mark comments as done to avoid forgetting about them)

src/lib/elementary/efl_ui_active_view_container.eo
60

New docs:

Packs a new view at the position indicated by @active_index (0 by default).

This is the same behavior as a push operation on a stack.

An animation might be triggered to make the new active view come into position.
69

No new docs needed, this was already popping from active_view.

segfaultxavi added inline comments.Jun 18 2019, 3:55 AM
src/lib/elementary/efl_ui_active_view_container.eo
79

New docs:

This Future gets resolved when any transition animation finishes and the popped view is ready for collection.
If there is no animation, the Future resolves immediately.
If $deletion_on_transition_end is $true then this widget will destroy the popped view and the Future will contain no Value. Otherwise, the caller becomes the owner of the view contained in the Future and must dispose of it appropriately.

@bu5hm4n @segfaultxavi

I am sorry for saying this now.

But I discussed about this topic with my colleague developers and the major feedback was that push/pop is not required for this case.

In C#, developers normally do not care about deleting an object (due to garbage collector). Even in C, developers hardly unpacks views (they just move the active index without unpacking views).

Moreover, current push/pop concept here is not exactly the push/pop concept of stack data structure. So it confuses developers too.

In short, how about not support push/pop?

Jaehyun_Cho added a comment.EditedJun 18 2019, 5:39 AM

@bu5hm4n @segfaultxavi

This may not related to this topic. ;)

But can we make pack() work like pack_end()? instead of pack_begin()?

Because if we think about multiple pages with scrolling, normally a new view is added to the right end.
(Other EFL containers do pack() like pack_end())

Moreover, pack_begin() causes all indices increase. If developer wants to maintain their views with index, then it would be inconvenient.

I know if pack() works like pack_begin(), then we do not need to call active_index_set if the active index is 0.
However, it seems that the concept which a new view is added to the beginning is not that common to developers.

Okay, we have had now 3 different approaches, and every single approach implemented 2 times.I seriously do not buy that this is confusing, where or why is it confusing? it is a dead simple push and pop that inserts things just like in a normal addressable stack data struct. What exactly is the reason ? where it is confusing ?

We are turning here in circles again and again and again. And each time the current implementation is confusing. Maybe we should reach a point where we say "get used to it" and continue with this ? I honestly have to say, every single time such a argument as "this is confusing" is done i am more and more loosing trust in the feedback i am getting here... Why should i now for example implement pack() like pack_end(), it may be considered confusing tomorrow ... and the progress is 0.

zmike added a comment.Jun 18 2019, 6:37 AM

@bu5hm4n @segfaultxavi

I am sorry for saying this now.

But I discussed about this topic with my colleague developers and the major feedback was that push/pop is not required for this case.

In C#, developers normally do not care about deleting an object (due to garbage collector). Even in C, developers hardly unpacks views (they just move the active index without unpacking views).

Moreover, current push/pop concept here is not exactly the push/pop concept of stack data structure. So it confuses developers too.

In short, how about not support push/pop?

I'm just catching up on this. It seems like there's maybe some uncertainty regarding these methods?

I think for now it would be best if we added push+pop as @beta methods. We can see how/if they get used and then decide whether to stabilize those methods (or remove them) just prior to the EFL release.

bu5hm4n updated this revision to Diff 22797.Jun 18 2019, 6:55 AM

add @beta to api, do docs

bu5hm4n marked 7 inline comments as done.Jun 18 2019, 6:55 AM

Okay, we have had now 3 different approaches, and every single approach implemented 2 times.I seriously do not buy that this is confusing, where or why is it confusing? it is a dead simple push and pop that inserts things just like in a normal addressable stack data struct. What exactly is the reason ? where it is confusing ?

We are turning here in circles again and again and again. And each time the current implementation is confusing. Maybe we should reach a point where we say "get used to it" and continue with this ? I honestly have to say, every single time such a argument as "this is confusing" is done i am more and more loosing trust in the feedback i am getting here... Why should i now for example implement pack() like pack_end(), it may be considered confusing tomorrow ... and the progress is 0.

First of all, I understand you. But I want to say something clear here.

We have been discussing how to improve this patch here. As we discussed we may find out something new and it can lead us to another direction.
If a solution is suggested once, then should we go to it even if we find out something more or we have some more opinion?
If this is not important widget, then it may time consuming but as you know this is one of the most important widgets and will be most frequently used widget. Therefore, if any bug or inconvenience of method usage exists, then it will become serious problem.

Once a patch or widget is submitted to upstream, then even if it is a beta thing or not, it makes many developers start implementing demo applications or sample applications. If a modification is required, then their codes should be modified again and again.
Moreover, if this container is released, then we need to maintain this container and also deal with feedback from app developers.
I am sorry to give you many comments and feedback here to modify this patch again which may look circular. But I want to conclude the best solution for this widget and I want to make less modification for app developers as much as possible.

Again, I am sorry I gave you many comments and feedback to make you modify this patch again. But I hope this container will be the most convenient container and be welcomed by app developers.

I am totally in favor of discussing that, and get feedback, but you are right now suggesting that i am going back to solution 1, which was considered *confusing*. Additionally, i feel like you do not have any example or usage for *not* having push or pop, this means every single time you want to know when the animation is finished you have to subscribe to the event, and unsubscribe from the event when it got called, which is an enormous code overhead, so if you showcase the solution of *do it yourself*, people will find it confusing as you do have to do two calls instead of pop, and a lot more in case of push.

I am strongly in favor of adding this *now* as beta API, if sample applications do like how this is API is done, then they will use it, and we can unbeta it. If not, we can still remove it again. And not include it at all. Does that sound like a plan ?

zmike added a comment.Jun 18 2019, 8:55 AM

As @bu5hm4n has said, I think it's a clear indicator that the API is useful enough to keep if application developers are interested in using it during this evaluation period.

I am totally in favor of discussing that, and get feedback, but you are right now suggesting that i am going back to solution 1, which was considered *confusing*. Additionally, i feel like you do not have any example or usage for *not* having push or pop, this means every single time you want to know when the animation is finished you have to subscribe to the event, and unsubscribe from the event when it got called, which is an enormous code overhead, so if you showcase the solution of *do it yourself*, people will find it confusing as you do have to do two calls instead of pop, and a lot more in case of push.

I am strongly in favor of adding this *now* as beta API, if sample applications do like how this is API is done, then they will use it, and we can unbeta it. If not, we can still remove it again. And not include it at all. Does that sound like a plan ?

Ok, let's go with the current implementation (no more modification).

About "confusing" I only said that

Moreover, current push/pop concept here is not exactly the push/pop concept of stack data structure. So it confuses developers too.

As you know, stack data structure has "top" so stack data structure has a direction from bottom to top.
push() adds a data to the top and pop() deletes a data from the top.
If active index is not 0, then our push and pop are not the same with the original "push" and "pop" in the stack data structure because our push and pop adds and removes at the active index view.
If active index is not 0, then this container is not stack anyway. As you know, this container is basically based on list structure not stack structure. I pointed out this.

Instead of our push(), we can do pack_at to the current active index. And instead of our pop(), we can do unpack the current active index.
We can use animation_enabled property of view manager to show animation or not.

The only thing left is removal of the popped view.
As I said above, removing view case would be rare case. Instead of removing view, the views are kept and used again later. So I thought that we may leave this removal case.

BTW, let's go with the current implementation (no more modification) and let's use the evaluation period.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 18 2019, 10:44 PM
Closed by commit rEFL0a4beb291d83: efl_ui_stack: remove! (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain Why
This revision was automatically updated to reflect the committed changes.