Page MenuHomePhabricator

interface_scrollable: Unify basis of calculation of page_get logic
ClosedPublic

Authored by akanad on Jan 10 2017, 3:36 AM.

Details

Summary

If rtl mode is set, current_page_get api should return reversed page number.
To do that, make x position x-axis reversed before page calculating.

Also bring_in and page_show should show the reversed page in rtl mode.
This patch modify the functions to support that.

Lastly, scroller should be scrolling based on the right edge of the page.

This patch is a combination of the patches(D4559,D4560)

Test Plan
  1. Run scroller test on elementary_test
  2. Turn ui mirrored mode on
  3. Manipulate scroller in various ways
    • It should scroll proper position when you click next or prev btn.

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.
akanad updated this revision to Diff 10494.Jan 10 2017, 3:36 AM
akanad retitled this revision from to interface_scrollable: Unify basis of calculation of page_get logic.
akanad updated this object.
akanad edited the test plan for this revision. (Show Details)
akanad added reviewers: woohyun, z-wony, taxi2se.
z-wony edited edge metadata.Jan 10 2017, 7:37 AM

I think it should fix with show()/bring_in() APIs.
Because, they still showing a page which is calculated legacy way.
As a result, someone calls page_show() using page_get()'s result, they are conflicted.

Like this.

elm_scroller_current_page_get(sc, &page_x, &page_y);
elm_scroller_page_bring_in(sc, --page_x, page_y);

test plan:

  1. $elementary_test scroller
  2. Turn on "UI-Mirroring" to "On" from base window.
  3. Click 'prev page' or 'next page'

Please check odd behavior after this patch.

(※ You must create 'Scroller' window and then turn on mirroring switch.
      If you 'turn on mirroring switch' and then create 'Scroller' window,
      mirrored mode not be applied. it's another legacy bug.)
akanad updated this revision to Diff 10500.Jan 10 2017, 7:53 PM
akanad edited edge metadata.

I forgot to use existing api.
I guess this is more suitable for this patch.

Anyway, Thanks z1.
I've revised what you said, and I finally got conclusion.
the conclusion is that page_bring_in and page_show are wrong.
they just seem like working properly.

I am going to write another patchs soon. then comment on here.

cedric requested changes to this revision.Jan 12 2017, 4:29 PM
cedric added a reviewer: cedric.

Hum, correct me if I am wrong, but the two other patch are actually to be combined to have this patch working properly. Right ? If this is the case and the test case is the one described in the comment. Could you merge all 3 patchs into just one then ?

This revision now requires changes to proceed.Jan 12 2017, 4:29 PM
akanad updated this revision to Diff 10520.Jan 12 2017, 10:02 PM
akanad edited edge metadata.

I squashed related other commits into this commit.

akanad updated this object.Jan 12 2017, 10:07 PM
akanad edited the test plan for this revision. (Show Details)
akanad edited edge metadata.
jpeg added a comment.Jan 15 2017, 9:24 PM

This patch improves the test case.

But if you enable loop in X axis, and press next/prev you'll see lots of bad things:

  • Prev page stops at page 0
  • The proxy doesn't work and you see a wide grey area where it's supposed to loop.
jpeg accepted this revision.Jan 15 2017, 9:25 PM
Closed by commit rEFLfbad285eca8b: interface_scrollable: Unify basis of calculation of page_get logic (authored by WhiskyKiloSq, committed by Jean-Philippe Andre <jp.andre@samsung.com>). · Explain WhyJan 15 2017, 9:49 PM
This revision was automatically updated to reflect the committed changes.