Page MenuHomePhabricator

efl.text_style
Closed, ResolvedPublic

Description

| |interface Efl.Text_Style @beta
| |├ (P) text_color
| |├ (P) text_background_type
| |├ (P) text_background_color
| |├ (P) text_underline_type
| |├ (P) text_underline_color
| |├ (P) text_underline_height
| |├ (P) text_underline_dashed_color
| |├ (P) text_underline_dashed_width
| |├ (P) text_underline_dashed_gap
| |├ (P) text_secondary_underline_color
| |├ (P) text_strikethrough_type
| |├ (P) text_strikethrough_color
| |├ (P) text_effect_type
| |├ (P) text_outline_color
| |├ (P) text_shadow_direction
| |├ (P) text_shadow_color
| |├ (P) text_glow_color
| |├ (P) text_secondary_glow_color
| |├ (P) text_gfx_filter
| |├ (P) text_all_styles
| |├ (M) text_style_apply
bu5hm4n created this task.May 3 2019, 11:14 AM
bu5hm4n triaged this task as TODO priority.
zmike moved this task from Backlog to Needs experts on the efl: api board.Jun 12 2019, 7:38 AM
zmike added a comment.Oct 1 2019, 9:20 AM

The proposed rework:

interface Efl2.Text.Style.Properties
├ (P) foreground_color
├ (P) background_type
├ (P) background_color
├ (P) underline_type
├ (P) underline_color
├ (P) underline_height
├ (P) underline_dashed_color
├ (P) underline_dashed_width
├ (P) underline_dashed_gap
├ (P) underline2_color
├ (P) strikethrough_type
├ (P) strikethrough_color
├ (P) effect_type
├ (P) outline_color
├ (P) shadow_direction
├ (P) shadow_color
├ (P) glow_color
├ (P) glow2_color
├ (P) gfx_filter
├ (P) halign_auto_type
├ (P) halign
├ (P) valign
├ (P) line_spacing
├ (P) line_spacing_factor
├ (P) line_height
├ (P) line_height_factor
├ (P) tab_width
zmike added a comment.EditedOct 1 2019, 9:27 AM
├ (P) underline2_color
├ (P) glow2_color

These seem like they should be renamed to something clearer.

The changes are:

  • rename the color properties (good imo)
  • add these properties
├ (P) halign_auto_type

I think the idea here is that you have the option of 1) disabling it 2) respecting the internal property value from being set in API 3) autodetecting it based on locale. I'm not sure why 3 exists, however, since it seems like 2 should be applied based on the locale? Confusing.

├ (P) halign
├ (P) valign

These properties seem duplicated around a few times in the text APIs; I'm not sure I see a need for this beyond using the content_align property at a higher level unless I'm missing something?

├ (P) line_spacing

This is pixels?

├ (P) line_spacing_factor
├ (P) line_height

This is pixels?

├ (P) line_height_factor
├ (P) tab_width

Should probably be uint type?

ali.alzyod updated the task description. (Show Details)Dec 20 2019, 12:57 AM
ali.alzyod moved this task from Needs experts to Evaluating on the efl: api board.

This is should be ready to stabilize, any comments ?

The comments from mike above still look relevant, and unsolved, so i guess that needs solving first.

ali.alzyod added a comment.EditedDec 24 2019, 3:53 AM

The comments from mike above still look relevant, and unsolved, so i guess that needs solving first.

First:
I do not understand why @zmike talks about efl.text.format in this task

Second:
I can see only complains about:
├ (P) underline2_color
├ (P) glow2_color
@segfaultxavi @zmike how about underline_second_color and glow_second_color

The comments from mike above still look relevant, and unsolved, so i guess that needs solving first.

First:
I do not understand why @zmike talks about efl.text.format in this task

Second:
I can see only complains about:
├ (P) underline2_color
├ (P) glow2_color
@segfaultxavi @zmike how about underline_second_color and glow_second_color

I think a name telling *what* 2 means is more applicable.

it literally means second (the second color used), but I am open for any suggestions

@woohyun @segfaultxavi

I am thinking of moving these two methods (Efl.Canvas.TextBlock.Style_Apply and Efl.Canvas.TextBlock.All_Styles_Get) from Canvas.TextBlock into this interface, So both Canvas.TextBlock and Ui.TextBox can use it. what do you think ?

@ali.alzyod

I think it looks proper change. I agree with your opinion.

ali.alzyod updated the task description. (Show Details)Dec 31 2019, 12:41 AM

@zmike @segfaultxavi @bu5hm4n
underline2_color -> second_underline_color
glow2_color -> second_glow_color

I like both "2" and "second" as a part of property name. How do you think ?

Except this, I think the rest looks good.

I prefer secondary to second (that's why I used that word in the docs).
I definitely dislike using numbers in anything API-related.

I commented about moving the style_apply methods in D10991.

ali.alzyod updated the task description. (Show Details)

D11024 will rename glow2 and underline2 into secondary_underline and secondary_glow

Another opinion on this task ?

I think this looks good now :)

Ok. Let's move this to "stabilized'. Thanks !

woohyun moved this task from Evaluating to Stabilized on the efl: api board.Jan 8 2020, 8:18 PM