Page MenuHomePhabricator
Differential D8686

elm_scroller: make loop_set/get as generic API
ClosedPublic
Actions

Authored by shilpasingh on Apr 23 2019, 4:32 AM.

Details

Reviewers
zmike
eagleeye
bu5hm4n
Commits
rEFL49f22b60e595: elm_scroller: make loop_set/get as generic API
Summary

elm_scroller_loop_set cannot be used for other widgets except scroller,
the API should be generic as this functionality can be implemented by other widgets as well.

Test Plan

test looping functionality by calling elm_scroller_loop_set
Signed-off-by: Shilpa Singh <shilpa.singh@samsung.com>

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.
shilpasingh created this revision.Apr 23 2019, 4:32 AM
Herald added a project: efl. · View Herald TranscriptApr 23 2019, 4:32 AM
Herald added subscribers: committers, reviewers, cedric. · View Herald Transcript

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

shilpasingh requested review of this revision.Apr 23 2019, 4:32 AM
shilpasingh updated this revision to Diff 21558.Apr 23 2019, 4:39 AM

updated reviewer

shilpasingh edited the test plan for this revision. (Show Details)Apr 23 2019, 4:41 AM
shilpasingh added reviewers: zmike, eagleeye.
bu5hm4n requested changes to this revision.Apr 25 2019, 4:58 AM
bu5hm4n added a subscriber: bu5hm4n.

I am wondering a bit about this revision. This is a legacy API, shouldn't something like this be used in the new scroller class ? Otherwise this feature will be gone and or unable to be used on new widgets.

Additionally, the definition of the actual header API is missing.

This revision now requires changes to proceed.Apr 25 2019, 4:58 AM
shilpasingh added a comment.Apr 25 2019, 5:57 AM
In D8686#158735, @bu5hm4n wrote:

I am wondering a bit about this revision. This is a legacy API, shouldn't something like this be used in the new scroller class ? Otherwise this feature will be gone and or unable to be used on new widgets.

Additionally, the definition of the actual header API is missing.

we already have a legacy API elm_scroller_loop_set but this API has not been implemented in a generic manner, that means I cannot call elm_scroller_loop_set on widgets like genlist, toolbar etc.
this change will make sure elm_scroller_loop_set becomes a generic API.
This API will work like policy_set, page_size_set, bounce_set APIs, these are not yet added as part of efl_ui_scroller.c

Legacy API itself is enhanced to be generic currently, in future when all the above legacy APIs are ported, then loop_set/get also will get ported but as a generic API
now calling elm_scroller_loop_set on say genlist will cause crash. Looping mechanism can be widget specific, every scrollable widget might implement its own looping mechanism like genlist then
a call elm_scroller_loop_set(genlist, ...); should work successfully.

shilpasingh added a comment.Apr 28 2019, 11:51 PM
In D8686#158759, @shilpasingh wrote:
In D8686#158735, @bu5hm4n wrote:

I am wondering a bit about this revision. This is a legacy API, shouldn't something like this be used in the new scroller class ? Otherwise this feature will be gone and or unable to be used on new widgets.

Additionally, the definition of the actual header API is missing.

we already have a legacy API elm_scroller_loop_set but this API has not been implemented in a generic manner, that means I cannot call elm_scroller_loop_set on widgets like genlist, toolbar etc.
this change will make sure elm_scroller_loop_set becomes a generic API.
This API will work like policy_set, page_size_set, bounce_set APIs, these are not yet added as part of efl_ui_scroller.c

Legacy API itself is enhanced to be generic currently, in future when all the above legacy APIs are ported, then loop_set/get also will get ported but as a generic API
now calling elm_scroller_loop_set on say genlist will cause crash. Looping mechanism can be widget specific, every scrollable widget might implement its own looping mechanism like genlist then
a call elm_scroller_loop_set(genlist, ...); should work successfully.

Also as elm_scroller.eo is already removed, I added the required changes in elm_scroller.eo.c directly
@bu5hm4n Please reply to my comment so that I can take appropriate action and raise an updated patch

bu5hm4n accepted this revision.Apr 29 2019, 12:08 AM

This looks fine i guess. I got distracted by something else. Sorry.

For further revisions: If you disagree with the reviewers comment, then you can set the revision into the state "Request Review" then it will again show up in the filter for revision that need attention.

This revision is now accepted and ready to land.Apr 29 2019, 12:08 AM
Closed by commit rEFL49f22b60e595: elm_scroller: make loop_set/get as generic API (authored by shilpasingh, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyApr 29 2019, 12:11 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 21763

View Options

src/lib/elementary/elm_scroller.c

Loading...
View Options

src/lib/elementary/elm_scroller_eo.c

Loading...