This is the first patch introducing efl_ui_pager into elementary.
|Unit Tests Skipped|
This looks very much like an interface. Or @pure_virtual functions at least. And the constructor should not be there if it's empty.
Not Efl.Object but Efl.Ui.Pager or whatever you really want.
This is not necessary. After bind the transition object should be able to listen to geometry (resize/move) events on its target.
This entire file is not required...
Please use the existing functions in eina. Use Eina_Rect or Eina_Rectangle internally.
t = abs(pd->move)
with Eina_Rect you can then do position_set(obj, pi->rect.pos)
no need for !! here :)
I think all the viewport information should be here. Including clippers and geometry.
pi = calloc(1, sizeof(*pi))
ouch. please make the code dynamic... was is this arbitrary value of 4?
please don't use group_add in new widgets... constructor and finalize should do just fine
same remark for group_del
calloc, no cast, sizeof(*pi)
fixme: should be stack above
maybe this interface should return false? (just wondering your opinion)
this should use the same type as other sizes: Eina.Size2d
i wonder if padding should be scalable or not? and maybe pass a bool flag to say scalable or not
loop might need to be an enum:
called for every change when dragging? or when the page changed?
specific to the transition
Okay this is hard to review somehow Phab doesn't give me context :(
"packed" event belongs to the efl.pack interface. It should also have the packed object as event info, definitely not some private data.
Well that's not possible. Efl_Ui_Pager_Data is a private struct.
No no no you can't use a private data struct as event info, how would apps use it?
It remains as a class. I added some common code in _efl_page_transition_bind().
what about circular reference? build fails with Efl.Ui.Pager...
I moved the common data for efl_page_transition's subclasses to here.
I chose Eina_Rectangle to use eina_rectangles_intersect().. but Eina_Rectangle can't be passed as a parameter of efl_gfx_geometry_set() ... hmm.
pd->move is a double..
i'll think about it
we'll discuss this off-line
"updated" should remain internal.. I'll add page change event!
This breaks distcheck. Please add test_pager to EXTRA_DIST. Apply this patch:
diff --git a/data/elementary/objects/Makefile.am b/data/elementary/objects/Makefile.am index 1c98813325..a7c5ee2abf 100644 --- a/data/elementary/objects/Makefile.am +++ b/data/elementary/objects/Makefile.am @@ -6,6 +6,7 @@ elementary/objects/test_external.edc \ elementary/objects/test_masking.edc \ elementary/objects/test_prefs.edc \ elementary/objects/test_prefs.epc \ +elementary/objects/test_pager.edc \ elementary/objects/multip.edc \ elementary/objects/cursors.edc \ elementary/objects/font_preview.edc \
ewww... fixed it.
on my side, distcheck failed as follows:
|configure: error: Systemd dependency requested but not found|
I configured with --disable-systemd option,
maybe I can't see the right error message with --disable-systemd option?
yes, right. I only pushed codes blocking the event and didn't push the displaying part.. it's coming~!
actually I'm thinking about how pages should look like while loop is disabled, cause i'm not sure the "proxy" thing will work everytime...
what if a pager displays 5 pages in the viewport and user sets only 2 pages in it? then should it look like
[Page1] [Page2] [Page1] [Page2] [Page1] ...?
This feature very much needs a good test case, with real content (that has a min size), not just simple edje objects.
Same question with a single page :)
Yes, as we discussed, the proxy can be a future option. I think you need an enum in loop instead of bool.
How does it fail? It shouldn't fail. Circular refs are OK.
mark as @nonull
This needs documentation and a test case. Otherwise don't add the API. Does it actually make sense in the general case or only with the scroll page transition?
there's already padding in pack api. but it's 2d. what about scaling here?
You might want to compile with -Wfloat-equal.
Why this choice here?
Any reason for using legacy callbacks? Besides just habits...
i think this should call the evas intercept function. look at other implementations of size set
why don't you use Eina_Size2d internally?
you might want to verify size >= 0 (or > 0?)
I tried everything (?) and still get this..
In file included from ../../src/lib/elementary/Elementary.h:287:0,
I'm fixing your inline comments
no, not the comments, the code.
and besides simple code fixing, I want to talk about the concept and logic soon.. because I think it can be much better than now.. !
For circular dependency, you have to define the type in C, otherwise the C code can not compile. But eolian doesn't complain. Example in Ector_GL.h:
#ifndef _ECTOR_GL_SURFACE_EO_CLASS_TYPE #define _ECTOR_GL_SURFACE_EO_CLASS_TYPE typedef Eo Ector_Cairo_Surface; #endif
just randomly chose one option before opening this as an API. how do you think?
This "scroll" means scrolling interaction, so it is a general property.
I'll fix this later... I'm not sure whether this should remain as a common property or not..