Page MenuHomePhabricator

efl.gfx.entity: combine show/hide events into visibility,changed
ClosedPublic

Authored by zmike on Feb 25 2019, 7:38 AM.

Details

Summary

this requires some internal hackery to preserve legacy compatibility
and correctly translate the single new event into two legacy events

ref T7558

Depends on D8018

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.
zmike created this revision.Feb 25 2019, 7:38 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/

zmike requested review of this revision.Feb 25 2019, 7:38 AM
segfaultxavi requested changes to this revision.Feb 25 2019, 8:04 AM
segfaultxavi added a subscriber: segfaultxavi.

Builds without warnings and a random sample of tests continue working as expected.
Just those minor inline comments.

src/lib/elementary/efl_ui_stack.c
261

I'd also rename the method to _pop_content_visibility_changed_cb. Otherwise it's confusing.

src/lib/evas/canvas/evas_object_inform.c
6–9

What happened to move and resize events? Why do they disappear here?

This revision now requires changes to proceed.Feb 25 2019, 8:04 AM
zmike requested review of this revision.Feb 25 2019, 9:01 AM
zmike added inline comments.
src/lib/elementary/efl_ui_stack.c
261

It's for the purpose of catching the hidden state though, so imo this still fits.

src/lib/evas/canvas/evas_object_inform.c
6–9

Those shouldn't have been pushed in to begin with, so the fact that I was able to just rename dead code and put it to use is a happy accident.

bu5hm4n requested changes to this revision.Feb 25 2019, 9:14 AM
bu5hm4n added a subscriber: bu5hm4n.
bu5hm4n added inline comments.
src/lib/elementary/efl_ui_stack.c
266

You might hate me now, but this is not obvious, that this means that the object is beeing shown, can you just call efl_gfx_visibility_get ? that would IMO improve this quality wise a lot.

This revision now requires changes to proceed.Feb 25 2019, 9:14 AM
zmike requested review of this revision.Feb 25 2019, 9:35 AM
zmike added inline comments.
src/lib/elementary/efl_ui_stack.c
266

I could, but that would defeat the purpose of passing the state with the event since we're trying to reduce eo indirection overhead.

segfaultxavi accepted this revision.Feb 25 2019, 9:39 AM

I rest convinced.

zmike updated this revision to Diff 19669.Feb 25 2019, 10:54 AM
zmike edited the summary of this revision. (Show Details)

rebase

This revision was not accepted when it landed; it landed in state Needs Review.Feb 25 2019, 11:01 AM
This revision was automatically updated to reflect the committed changes.