Page MenuHomePhabricator

Refactor scrollable mixin
Open, HighPublic

Description

A big portion of scrollable interface should be internal and properly refactorized with all our use of it.

cedric created this task.Mar 30 2017, 2:40 AM
cedric added a subscriber: woohyun.Apr 4 2017, 4:15 PM
akanad added a subscriber: akanad.May 19 2017, 2:34 AM

We(@eunue @eagleeye) are under discussion about "scrollable things".
We made a little humble proposal of those things so that I'm posting here.

Key points of the proposal are like below.

  1. interface which is a minimal subset of scrollable things
  2. minimal concrete class which implements the interface
  3. do not use inherit to make scrollable widgets, but have concrete class
  4. clear separation between 'scroll' and 'content'

I'm not sure that this concept is feasible and suitable on EFL so that I need your opinion.
Thanks

eunue added a comment.Jun 9 2017, 3:18 AM

Should each property in Elm.Interface_Scrollable be a part of Efl.Ui.Scrollable interface?

Property nameYES or NODescription
contentNOSetter only. This should remain internal.
content_sizeNOGetter only. This simply returns the geometry of the content object. Is not a property of scrollable widgets.
content_viewport_geometryNOGetter only. Pan object’s geometry. This should remain internal.
objectsNOThis should remain internal.
extern_panNOThis should remain internal.
gravityYESActive when the size of the content or pan changes dynamically. Need to change name?
policyYESIs a property for every scrollable widgets. Need to change name => “scrollbar_mode”?
single_directionNOIs not a property of every scrollable widgets. Widgets except elm_scroller don’t need this. Or do they…? Most of scrollable widgets are one direction.
loopNOIs not a property of every scrollable widgets. Widgets except elm_scroller don’t need this.
wheel_disabledNOThis is not subject to API control. Should be part of the configuration.
bounce_allowTBDedge_effect?
bounce_animator_disabledNO
momentum_animator_disabledNO
movement_blockTBDThese properties should be considered with the properties in elm_widget.
freezeTBD
holdTBD
repeat_eventsTBD
step_sizeTBDMaybe only for elm_scroller? Configuration?
content_regionYES“content_region” is meaningless. => “content_position”
mirroredNOThis is widget property.
page_sizeTBDSee step_size.
page_scroll_limitTBD
page_snap_allowTBD
pagingTBD
last_pageTBD
current_pageTBD
page_relativeTBD
woohyun assigned this task to eunue.Jun 20 2017, 1:05 AM
cedric raised the priority of this task from TODO to High.Jul 10 2017, 3:23 PM

I am sadly not really versed in the scroller interface and its use. I am just wondering if we want to make effect part of its API (gravity, bounce and so on) or if there shouldn't be a way to expand the behavior and enable animation some how maybe by a delegated class ? Something that would interact with the animation framework maybe ? I am throwing idea here, nothing else. Let me know what you think.

eagleeye added a comment.EditedJul 11 2017, 3:10 AM


Each scrollable widget has "Efl.Ui.Scroll.Manager" instance, "Efl.Ui.Scroll.Manager" implements "Efl.Ui.Scrollable".

eagleeye added a comment.EditedJul 18 2017, 3:09 AM

enum Efl.Ui.Scroll.Single_Direction
{

none = 0, [[Scroll every direction]]
soft, [[Scroll single direction if the direction is certain]]
hard, [[Scroll only single direction]]
last  [[Sentinel value to indicate last enum field during iteration]]

}

interface Efl.Ui.Scrollable ()
{

   [[Efl UI scrollable interface]]
   event_prefix: efl_ui;
   methods {
      @property content_pos {
         [[The content position]]
	 set {
	 }
	 get {
	 }
 	 values {
            x: int;
	    y: int;
	 }
      }
      @property content_size {
         [[The content size]]
	 get {
	 }
	 values {
	    w: int;
	    h: int;
         }
       }
      @property viewport_geometry {
         [[The viewport geometry]]
	 get {
	 }
	 values {
            x: int;
	    y: int;
	    w: int;
            h: int;
	 }
      }
      @property single_direction {
         set {
         }
         get {
         }
         values {
            single_dir: Efl.Ui.Scroll.Single_Direction; [[The single direction scroll policy]]
         }
      }
      @property bounce_allow {
         [[Bouncing behavior]]
         set {
         }
         get {
         }
         values {
            horiz: bool; [[Horizontal bounce policy.]]
            vert: bool; [[Vertical bounce policy.]]
         }
      }
      @property freeze {
         [[Freeze property]]
         get {
	 }
         set {
         }
         values {
            freeze: bool; [[$true if freeze, $false otherwise]]
         }
      }
      @property hold {
         [[Hold property]]
         get {
	 }
         set {
         }
         values {
            hold: bool; [[$true if hold, $false otherwise]]
         }
      }
      bring_in {
         params {
            @in x: int; [[X coordinate of the region]]
            @in y: int; [[Y coordinate of the region]]
            @in w: int; [[Width of the region]]
            @in h: int; [[Height of the region]]
	    @in time: double;
	    @in type: int;
         }
      }
      show {
         params {
            @in x: int; [[X coordinate of the region]]
            @in y: int; [[Y coordinate of the region]]
            @in w: int; [[Width of the region]]
            @in h: int; [[Height of the region]]
         }
      }
   }
   events {
	  scroll,start;
          scroll; 
	  scroll,stop;
	  scroll,up;
	  scroll,down;
	  scroll,left;
	  scroll,right;
	  edge,up;
	  edge,down;
	  edge,left;
	  edge,right;
          scroll,anim,start; 
          scroll,anim,stop; 
          scroll,drag,start; 
          scroll,drag,stop; 
   }

}

jpeg reassigned this task from eunue to eagleeye.Dec 11 2017, 6:05 PM

The current patch is at D5222

jpeg added a comment.Jan 18 2018, 12:55 AM

Widget still has 4 unclear methods defined:

/* FIXME: Scroll API. Not sure how those APIs should be exposed with
 * the new scrollable API. */
scroll_hold_push {
   [[Push scroll hold]]
}
scroll_hold_pop {
   [[Pop scroller hold]]
}
scroll_freeze_push {
   [[Push scroller freeze]]
}
scroll_freeze_pop {
   [[Pop scroller freeze]]
}

I assume the following are acceptable:

@property on_show_region_hook @protected {
   [[Hook function called when the @.show_region is changed.

     See also @.show_region.
   ]]
   set {}
   values {
      func: Efl.Ui.Scrollable_On_Show_Region @nullable; [[Region hook function]]
   }
}
@property show_region @protected {
   [[Region inside the widget to show.

     See also @.on_show_region_hook.
   ]]
   set {
      [[Request parent scrollers to pan around so that this region
        of the widget becomes visible.

        If $force is $true this will trigger scroller changes and
        the @.on_show_region_hook to be called even if the region is
        unchanged.
      ]]
      values {
         region: Eina.Rect; [[The region to show.]]
         force: bool; [[Set to $true to force show even if unchanged.]]
      }
   }
   get {
      [[Returns the current region to show.]]
      values {
         region: Eina.Rect; [[The region to show.]]
      }
   }
}
eagleeye added a comment.EditedFeb 7 2018, 4:07 AM

scroll_hold_push/pop and scroll_freeze_push/pop are necessary. because slider and entry widget use these APIs.
When slider is dragging by user and slider's parent is scroller, scroller will not be moved. so scroll_freeze_push/pop is used.

So I add patch, https://phab.enlightenment.org/D5796

jpeg added a comment.Feb 7 2018, 6:49 PM

Yes those APIs are necessary, but they are still "not clean" in Efl.Ui.Widget, see T5363. They must be well documented, and it must be clear whether they should be in Widget or only in the Scrollable interface.

zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:54 AM
bu5hm4n edited projects, added efl: widgets; removed Restricted Project.Jun 11 2018, 9:15 AM
zmike added a subscriber: zmike.

A #Goal ticket should not be set to a milestone.

@eagleeye

Could you let me know the progress of this task if it is done?

@eagleeye

Could you let me know the progress of this task if it is done?

This task is almost done except define step_size and page size, apply focus logic.

zmike added a comment.Sep 3 2019, 12:04 PM

This seems irrelevant now?