Page MenuHomePhabricator

elm_panel: set hidden property when panel is close/open
ClosedPublic

Authored by taxi2se on Mar 6 2017, 11:48 PM.

Details

Summary
  • elm_panel has a property named hidden which stores open/close status.
  • This is updated when:
    1. bring_in animation is done(anim_stop_cb).
    2. mouse_up on panel.
    3. API is called. (elm_panel_toggle, elm_panel_hidden_set)
  • In case 3, API changes hidden, and starts bring_in animation which will call anim_stop_cb() which will update hidden again.
  • If bring_in animation is canceled (eg: sizing_eval), anim_stop_cb will be called and calculate hidden status which will not guarantee updated hidden state by APIs.
Test Plan
  1. Call any APIs which will call elm_layout_sizing_eval(panel) right after calling elm_panel_toggle()/elm_panel_hidden_set().
  2. Delete content of panel during "toggled" cb.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3335
Build 3400: arc lint + arc unit
taxi2se created this revision.Mar 6 2017, 11:48 PM


This is test of the problem case.

taxi2se added a subscriber: conr2d.
cedric requested changes to this revision.Mar 7 2017, 2:17 PM

Could you make the test part of the patch by extanding elementary_test ?

This revision now requires changes to proceed.Mar 7 2017, 2:17 PM
taxi2se requested review of this revision.Mar 8 2017, 3:01 AM
taxi2se edited edge metadata.

In my opinion, this case is not desirable enough to be exposed to elementary_test.
The problem case in here happens when elm_layout_sizing_eval() is called during toggle animation.
This will stop toggle animation(bring_in) and skip the animation by calling pos_set.
In normal case, it is hard to change panel during toggle animation.

Well, the purpose of a test that doesn't happen normally is so that when a developer later modify this code without having an idea of that said case, he can get it detected by just going through the example. Otherwise expect your bug to happen again in the future and have to fix it again.

jpeg edited edge metadata.Apr 19 2017, 10:00 PM

What @cedric said. Please add a test case... Maybe just modify the existing test case with an extra button, for instance.
Anyway I don't understand the provided test case. What bug am I supposed to see?

jpeg requested changes to this revision.Apr 19 2017, 10:01 PM
This revision now requires changes to proceed.Apr 19 2017, 10:01 PM
eunue edited edge metadata.Apr 27 2017, 2:51 AM

@taxi2se Can you please add a test case as @cedric and @jpeg said?
I agree that this case is not perfectly suitable for elementary_test for now,
but it needs to be developed into a decent test suite in the future.

taxi2se updated this revision to Diff 11432.May 26 2017, 4:26 AM
taxi2se edited edge metadata.
jpeg accepted this revision.Jun 8 2017, 12:11 AM

Honestly I'm not reviewing all the details in this patch. The test case is good. Thanks!

Closed by commit rEFL9dd997f38932: elm_panel: set hidden property when panel is close/open (authored by taxi2se, committed by Jean-Philippe Andre <jp.andre@samsung.com>). · Explain WhyJun 8 2017, 12:48 AM
This revision was automatically updated to reflect the committed changes.