Page MenuHomePhabricator

efl_ui_text: scroller mode clean up
ClosedPublic

Authored by ali.alzyod on Dec 29 2019, 6:58 AM.

Details

Summary

simplify code, and reduce calls:

It is only required to set scroll mode in two cases:

  • When creating scroller
  • When changing multiline property

Diff Detail

Repository
rEFL core/efl
Branch
arcpatch-D10981
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15193
Build 10478: arc lint + arc unit
ali.alzyod created this revision.Dec 29 2019, 6:58 AM

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/

ali.alzyod requested review of this revision.Dec 29 2019, 6:58 AM
ali.alzyod edited the summary of this revision. (Show Details)Dec 29 2019, 7:03 AM
ali.alzyod added reviewers: woohyun, zmike, bu5hm4n.
woohyun added inline comments.Dec 29 2019, 10:22 PM
src/lib/elementary/efl_ui_textbox.c
2129

I cannot find out where this function is called ..... Am I missing something ?

ali.alzyod added inline comments.Dec 29 2019, 10:57 PM
src/lib/elementary/efl_ui_textbox.c
2129

this is overriding for Efl.Text_Format.multiline {set;} in efl.textbox

segfaultxavi added inline comments.
src/lib/elementary/efl_ui_textbox.c
2132

I do not understand this early return. How can I disable multiline then?

bu5hm4n requested changes to this revision.Dec 31 2019, 3:17 AM

This is indeed a good catch.
Can you also remove smanager from Efl_Ui_Internal_Text_Scroller_Data ? As this is not used anymore now.

src/lib/elementary/efl_ui_internal_text_scroller.c
140

Mhm, i am not a fan of this. The flow of value of mulitline seems to come from textbox, the internal scroller should not eval this value.
Additionally, when text_obj's multiline is changed, this is told to the scroller via scroller_mode_set so i think this is not needed ?

src/lib/elementary/efl_ui_textbox.c
2128

Can you add EOLIAN here ?

2132

@segfaultxavi this just returns when the values are the same, so you can disable mulitple by passing enabled=false, which will just work :)

src/lib/elementary/efl_ui_textbox.eo
109

Can you also remove this here?

This revision now requires changes to proceed.Dec 31 2019, 3:17 AM

Additionally, i think efl_ui_internal_text_scroller_mode_set must be set when sd->scroller is created.

ali.alzyod updated this revision to Diff 27902.Jan 2 2020, 12:17 AM
ali.alzyod marked 6 inline comments as done.
ali.alzyod edited the summary of this revision. (Show Details)

update

ali.alzyod updated this revision to Diff 27903.Jan 2 2020, 12:19 AM

remove empty line

bu5hm4n accepted this revision.Jan 2 2020, 3:31 AM

Thank you :)

This revision is now accepted and ready to land.Jan 2 2020, 3:31 AM
Closed by commit rEFL3cc8ea77fa5c: efl_ui_text: scroller mode clean up (authored by ali.alzyod, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyJan 2 2020, 3:42 AM
This revision was automatically updated to reflect the committed changes.
segfaultxavi added inline comments.Jan 2 2020, 3:16 PM
src/lib/elementary/efl_ui_textbox.c
2132

Apologies, I brainfarted.