Page MenuHomePhabricator

efl.ui.relative_layout
Closed, ResolvedPublic

Description

class Efl.Ui.Relative_Layout @beta
├ (P) relation_left
├ (P) relation_right
├ (P) relation_top
├ (P) relation_bottom
YOhoho created this task.Oct 16 2019, 10:24 PM
YOhoho triaged this task as TODO priority.
@property relation_left {
         keys {
            child: Efl.Object; 
         }
         values {
            target: Efl.Object; 
            relative: double;
         }
      }

You can set base line of each side based on target.
relative decides relative position on horizontal(left, right)/vertical(top, bottom) axis of the target.

@bu5hm4n @zmike @segfaultxavi

I hope this would be reviewed earlier than other widget classes because many people have been waiting for this :)
If you have any question on this, @YOhoho would give the best answer on it.

I have been playing with this widget, and written an example:
https://git.enlightenment.org/tools/examples.git/tree/reference/c/ui/src/ui_relative_layout.c?h=devs/xartigas/efl_ui_relative_layout
I think it is very easy to use and understand. I mastered it very quickly.
The documentation can be improved, but it already contains all the information, including default values. Well done.
If a child has a min width of 100 pixels and it is set to be 50% as wide as the container, then the container cannot be resized to less than 200 pixels. This was surprising to me, since the min width of the parent is being controlled by the children, which is very handy. Again, well done.

I have some comments, though:

  • A NULL target means the parent. This is very useful but it should be documented.
  • We have no other container called "layout" and this is a bit confusing. At the beginning I didn't know how to use this widget until I realized that it implements the Efl.Pack interface, and therefore children must be "packed" inside it, just like a regular container. Why don't we call this widget Efl.Ui.Relative_Container ?
  • Instead of target I would call it base or origin. "Target" sounds like the final position, and in this case it really is the starting position.
  • Instead of relative I would call it relative_offset. "Relative" on its own does not have much meaning. "Offset" is telling you that the widget will be offset (this is, "moved") from the starting position.

child can be a key or a value, I have no preference.
All four methods could be merged into one, and a new parameter added to specify the edge to modify. This would require creating an Efl.Ui.Edge enum. I have no strong preference.

In general, the widget works well and the API is simple and easy to use. Good work!

woohyun added a comment.EditedOct 29 2019, 2:47 AM

@segfaultxavi

Thanks for sharing your opinion.

  1. I also like "base" instead of "target"
  1. "Relative_Layout" come from other platforms' class name with the same usage. If, everyone else does not have any bad with "Relative_Container", we can go with that :) (I hope to get more candidates for this class's name)
  1. "relative_offest" ... Hmm. "offset" was used with pixel in EFL, so I don't think it's the best one. But ! I don't have better idea for now .. so need to think about that a bit more.
  1. "child" , "key", "value", "current", or "content". I think just keeping consistency with other classes would be good for this value for "keys".
  1. For better intuition, I like to go with current propeI hope this would be reviewed earlier than other widget classes because many people have been waiting for this :)

If you have any question on this, @YOhoho would give the best answer on it.rties. (instead of adding enum)

@YOhoho Could you share your idea on this feedback :) ?

zmike added a comment.Oct 29 2019, 6:35 AM

I agree that we should reserve the term "layout" for things related to layout (edje) in EFL, even though this may be confusing for other platforms.

Given that the values here are effectively "double value which means alignment" I'm wondering if we want to consider an efl.gfx double type for this (e.g., Efl.Gfx.Align) so we can be more explicit; this already has a number of places it can be used...

Given that the values here are effectively "double value which means alignment" I'm wondering if we want to consider an efl.gfx double type for this (e.g., Efl.Gfx.Align) so we can be more explicit; this already has a number of places it can be used...

That's actually not a bad idea.

"relative_offest" ... Hmm. "offset" was used with pixel in EFL, so I don't think it's the best one. But ! I don't have better idea for now .. so need to think about that a bit more.

Then relative_position or relative_anchor... But it needs a noun after relative, for sure :)

Efl.Gfx.Align has been landed in D10554 and D10555. I hope everybody is OK with it :)

  1. I also like "base" instead of "target"

+1 "base"

  1. "Relative_Layout" come from other platforms' class name with the same usage. If, everyone else does not have any bad with "Relative_Container", we can go with that :) (I hope to get more candidates for this class's name)

+1 "Relative_Container"

  1. "child" , "key", "value", "current", or "content". I think just keeping consistency with other classes would be good for this value for "keys".

Hmm.. i can't find a better name than "child".

Then relative_position or relative_anchor... But it needs a noun after relative, for sure :)

+1 "relative_position". Type name already describe this parameter is related to align.

  1. target -> "base"
  2. Relative_Layout -> "Relative_Container"
  3. keep "child" as it is
  4. relative -> "relative_position"

Any other opinions on this (and for the other definitions) ?

@segfaultxavi @zmike @bu5hm4n

I think this looks good now.
Is it ok to remove @beta ? or any other opinions ?

Looks good to me!

Ok ~ If there would be no more feedback in a day ~ I'll move this task to "stabilized".

Jaehyun_Cho moved this task from Backlog to Stabilized on the efl: api board.Sun, Dec 1, 10:03 PM
bu5hm4n closed this task as Resolved.Wed, Dec 4, 7:52 AM
bu5hm4n claimed this task.