Page MenuHomePhabricator

popup: fix popup sizing when scroll enabled.
ClosedPublic

Authored by netstar on Jul 4 2018, 9:51 AM.

Details

Summary

Force immediate calculate on main_layout after sizing hints set.

@fix T6886

Test Plan

Elementary_test: popup -> select scrollable -> use popup examples.

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.
netstar created this revision.Jul 4 2018, 9:51 AM
netstar requested review of this revision.Jul 4 2018, 9:51 AM
zmike added a comment.Jul 4 2018, 10:46 AM

Based on the surrounding code in that function, should there be an if (sd->main_layout) check before calling that?

This function can be expensive to call repeatedly...

netstar updated this revision to Diff 15459.Jul 4 2018, 10:52 AM

Let's not call this all the time.

zmike added a comment.Jul 4 2018, 10:54 AM

Seems smart. Is it always the case that this object exists in a scrollable popup?

netstar updated this revision to Diff 15460.Jul 4 2018, 11:01 AM

Yes you're right. was rushing :)

netstar updated this revision to Diff 15461.Jul 4 2018, 11:01 AM

Sorry, spaces :/

netstar updated this revision to Diff 15462.Jul 4 2018, 11:16 AM

Force recalc only when already doing a cancvas-wide recalc

netstar updated this revision to Diff 15463.Jul 4 2018, 11:28 AM

Set the flag to recalculate rather than forcing a full
recalculation.

zmike added a comment.Jul 4 2018, 4:09 PM

If this works 100% of the time that's fine, but generally you wouldn't check calculating if you're just setting the recalc flag; that function was added specifically so that you could know whether to set the flag or manually call recalc because if you are inside a calc then you want to trigger the calc in order to avoid an infinite calc loop. I'd be concerned about such a loop for this case if there was an unstable edje object somewhere in the chain...

netstar updated this revision to Diff 15465.Jul 5 2018, 12:46 AM

Just set flag to recalc. Don't check object is calculating.

Acting on comment from zmike.

Whatever you guys think. The latter seems simpler and less intensive. Let me know :)

zmike added a comment.Jul 5 2018, 11:43 AM

I think probably if should be if (calculating) smart_calc(main_layout) else need_recalculate_set(main_layout) just to avoid potential infinite loops which might not occur with the default theme.

stephenmhouston requested changes to this revision.Jul 5 2018, 12:13 PM
stephenmhouston added inline comments.
src/lib/elementary/elc_popup.c
506

Formatting. Spacing is off.

507

Formatting. Spacing is off.

This revision now requires changes to proceed.Jul 5 2018, 12:13 PM
netstar updated this revision to Diff 15475.Jul 5 2018, 12:16 PM

As of above comment from zmike.

I think that's ok @stephenmhouston

Very odd you commented as I just updated.

netstar marked 2 inline comments as done.Jul 5 2018, 12:18 PM

Ok.

netstar updated this revision to Diff 15477.Jul 5 2018, 12:20 PM

update the spacing.

zmike accepted this revision.Jul 6 2018, 6:32 AM
stephenmhouston accepted this revision.Jul 6 2018, 7:07 AM
This revision is now accepted and ready to land.Jul 6 2018, 7:07 AM
This revision was automatically updated to reflect the committed changes.