Page MenuHomePhabricator

Efl.Ui.Alert_Popup
Closed, ResolvedPublic

Description

class @beta Efl.Ui.Alert_Popup extends Efl.Ui.Popup
{
   [[EFL UI Alert Popup class]]
   methods {
      @property button {
         set {
            [[Set popup buttons.]]
         }
         keys {
            type: Efl.Ui.Alert_Popup_Button; [[Alert popup button type]]
         }
         values {
            text: string; [[Alert string on button]]
            icon: Efl.Object; [[Alert icon on button]]
         }
      }
   }
   implements {
      Efl.Object.constructor;
      Efl.Object.destructor;
      Efl.Part.part_get;
   }
   events {
      button,clicked: Efl.Ui.Alert_Popup_Button_Clicked_Event; [[Called when alert popup was clicked]]
   }
}

Action Item

  • Change of Event type & Removal of struct type
  • Write Tests. / Verify event emission / Veryfy funcitonal icon / str parts.
segfaultxavi triaged this task as TODO priority.
segfaultxavi updated the task description. (Show Details)

The event looks a bit overengeneered to me. Why passing a struct if its just a enum in the struct ? I think passing Efl.Ui.Alert_Popup_Button as event type is totally enough.

A different approach to the button API: What do you think of modelling it as parts ? So you have parts positive negative and user. And you can set text via efl_text on it or the icon via efl_content_set ?

Additionally:

  • This needs tests for testing the event emission, and the button setting

Thank you for the opinion.
I agree with you about event info. Regardless of the original intention (why event info is structure), it seems that no new information is required for alert popup's button clicked event.

So I also want to change event info from structure to enum but it does not work in C# because the current marshaling logic does not convert enum in event info correctly. (struct is converted well.)
So now I think I need to investigate how to solve the current marshaling logic and then I will change event info.

About parts, I think the current usage (@property button) is easier. Because we only need one button clicked event callback and we can simply indicates based on the button type (enum).
Moreover, as far as I see, efl parts are used if the part objects always exist. (e.g. background) But the alert popup's button may exist and may not exist.
The way how to unset alert popup's button becomes more complicated if we apply parts for the button.

So I think we keep the current @property button here.

I also think using the enum in the event info is good enough, and that using parts would complicate things too much.
If anybody needs a more flexible popup, he can always extend from Efl.Ui.Popup, no?

@segfaultxavi

Yes, since Efl.Ui.Alert_Popup simply adds its buttons and button clicked event to Efl.Ui.Popup, extending Efl.Ui.Popup is enough for developer who needs a more flexible popup.

@Jaehyun_Cho

I just wanted to ensure we have thought about button as part, so this is fine to me :)
So action items for this here are :

  • Change of Event type & Removal of struct type
  • Write Tests. / Verify event emission / Veryfy funcitonal icon / str parts.
bu5hm4n updated the task description. (Show Details)Jun 25 2019, 9:35 AM
zmike closed this task as Resolved.Sep 27 2019, 11:26 AM
zmike claimed this task.