Page MenuHomePhabricator

efl_ui_table: refactor layout_update
ClosedPublic

Authored by YOhoho on Jan 31 2019, 12:40 AM.

Details

Summary

There are three reasons to refactor layout_update of Efl.Ui.Table.

1. Inconsistency of hint behavior.

Some hint property is often not respected. for example, hint_min is ignored in
Table when it is used with hint_max even if hint_weight is 0. hint_aspect is
always ignored in Table.
The ambiguous behavior make it hard to layout widgets in container. of course,
we documented 'it's just a hint that should be used whenever appropriate.' but i
don't think it means that 'hint API is sometimes respected and we also don't
know when that API is respected.'. at least there is rule for consistent
behavior and we should be able to explain why a widget is located here and
why some hint property is ignored.

So, i'll suggest priority of hint property. this refactoring support following
priority.

  1. HintMin
  2. HintMin + HintAspect
  3. HintMargin
  4. HintMax
  5. HintAspect
  6. HintWeight, HintFill
  7. HintAlign

ref T5487
Please check with unit test D7840

2. To Enhance usability.

Efl.Ui.Table is using homogeneous mode of evas_table which have same columns,
rows size. but i think a table can generally change columns, rows size and
we can provide homogeneous mode option.(D7892)

In this patch

  • table columns(rows) min size is decided by maximum size among its cells

width(height) min size.

  • table columns(rows) weight is decided by maximum weight among its cells

horizontal(vertical) weight.

Also, pack_align is implemented. it is used if no item has a weight.

3. To remove internal evas_table.

This is low priority work. however, i guess is is necessary for lightweight
container widget. there are two size_hint callback to adjust table size and
efl_canvas_group_calculate is called twice when it is resized.
This patch is first step to remove internal evas_table.

Test Plan

make check
elementary_test -to 'efl.ui.table'

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.
YOhoho created this revision.Jan 31 2019, 12:40 AM
YOhoho requested review of this revision.Jan 31 2019, 12:40 AM
zmike added a subscriber: zmike.Jan 31 2019, 5:34 AM

Thanks for providing a unit test series with this. Can you add more detail to the commit log explaining what this refactor is intended to accomplish?

zmike requested changes to this revision.Jan 31 2019, 5:34 AM
This revision now requires changes to proceed.Jan 31 2019, 5:34 AM
YOhoho edited the summary of this revision. (Show Details)Jan 31 2019, 10:29 PM
YOhoho edited the test plan for this revision. (Show Details)
YOhoho edited the summary of this revision. (Show Details)
YOhoho edited the summary of this revision. (Show Details)
YOhoho updated this revision to Diff 19228.Feb 6 2019, 9:58 PM

clean up code

YOhoho updated this revision to Diff 19230.Feb 6 2019, 10:10 PM
YOhoho edited the summary of this revision. (Show Details)

update commit message

YOhoho updated this revision to Diff 19240.Feb 7 2019, 1:58 AM

remove needless variable and rename calc function

YOhoho updated this revision to Diff 19308.Feb 12 2019, 1:58 AM
YOhoho edited the summary of this revision. (Show Details)

implement pack_align

zmike added a comment.Feb 12 2019, 6:20 AM

What do you think about trying to unify some of the item sizing calcs between box and table? A lot of this code looks pretty similar to the refactored box code.

src/lib/elementary/efl_ui_table.c
157

This should probably not be adding more legacy code usage inside ui code?

259

legacy...

703

legacy...

YOhoho updated this revision to Diff 19438.Feb 15 2019, 2:51 AM
YOhoho edited the summary of this revision. (Show Details)

remove legacy codes

In D7841#140997, @zmike wrote:

What do you think about trying to unify some of the item sizing calcs between box and table? A lot of this code looks pretty similar to the refactored box code.

That's good to maintain the code.
Is there any good place for shared code?(efl_ui_container_layout.c or efl_ui_containet_helper.c, for example) or just including efl_ui_box_private.h in efl_ui_table_layout.c is better?

YOhoho updated this revision to Diff 19469.Feb 20 2019, 6:28 PM

unify duplicated code between box and table layout_update code.

Also, it remove direction check codes.

YOhoho updated this revision to Diff 19609.Feb 23 2019, 4:13 AM
YOhoho edited the summary of this revision. (Show Details)

rebase

zmike added a comment.Feb 25 2019, 4:22 AM
In D7841#140997, @zmike wrote:

What do you think about trying to unify some of the item sizing calcs between box and table? A lot of this code looks pretty similar to the refactored box code.

That's good to maintain the code.
Is there any good place for shared code?(efl_ui_container_layout.c or efl_ui_containet_helper.c, for example) or just including efl_ui_box_private.h in efl_ui_table_layout.c is better?

I think you can just make a new file for this? I'm going to land this series since it seems useful overall, but refactoring for deduplication is always beneficial!

zmike accepted this revision.Feb 25 2019, 4:22 AM
This revision is now accepted and ready to land.Feb 25 2019, 4:22 AM
YOhoho updated this revision to Diff 19693.Feb 26 2019, 3:21 AM

rename cell_count to cell_index

This revision was automatically updated to reflect the committed changes.