Page MenuHomePhabricator

Efl2.Input.Text
Open, TODOPublic

Description

From current devs/tasn/ifaces

interface Efl2.Input.Text
├ (P) autocapital_type
├ (P) prediction_allow
├ (P) input_hint
├ (P) input_panel_show_on_demand
├ (P) input_panel_language
├ (P) input_panel_layout_variation
├ (P) input_panel_return_key_disabled
├ (P) input_panel_layout
├ (P) input_panel_return_key_type
├ (P) input_panel_enabled
├ (P) input_panel_return_key_autoenabled
├ (M) input_panel_show
├ (M) input_panel_imdata_set
├ (M) input_panel_imdata_get
├ (M) input_panel_hide
zmike created this task.Oct 1 2019, 9:41 AM
zmike triaged this task as TODO priority.
zmike added a comment.Oct 1 2019, 9:52 AM

I'll preface this by saying that I'm not an expert on input panel needs (@woohyun and HQ experts on that) and am only reviewing the API based on a surface-level understanding of it and how cohesive it may be to the rest of the stabilized EFL API stack. This interface is for defining methods for allowing widgets to interact with input method modules.

interface Efl2.Input.Text
├ (P) autocapital_type

This name feels weird.

├ (P) prediction_allow

Shouldn't this just be text_prediction with the value being enable?

├ (P) input_hint

This is pretty ugly with such a huge enum, but I'm not sure how we could improve on it without making the API worse. Will require some thought.

This naming is excruciating. I'm wondering if it's worth condensing some of these properties into a bitflag enum since we already have a lot of that in this API.

├ (P) input_panel_show_on_demand
├ (P) input_panel_language
├ (P) input_panel_layout_variation

What even is this?

├ (P) input_panel_return_key_disabled
├ (P) input_panel_layout
├ (P) input_panel_return_key_type
├ (P) input_panel_enabled

Can't this just be input_panel with the value enabled?

├ (P) input_panel_return_key_autoenabled

├ (M) input_panel_show
├ (M) input_panel_hide

These should be combined into a input_panel_visibility property to be consistent with all other visibility-related APIs.

├ (M) input_panel_imdata_set
├ (M) input_panel_imdata_get

These should definitely be private and not in eo files.

zmike updated the task description. (Show Details)Oct 1 2019, 9:53 AM
tasn added a comment.Oct 1 2019, 1:18 PM

Sorry this is still just a copy paste from what was there before. I'm still waiting for feedback on these functions in T8226. They will only be changed after that.

@woohyun, do you fellas have any input on this? A lot of these functions need like they need some changes and you all have the expertise. I was hoping to get feedback in the other ticket, but let's just merge the discussion to here.

Edit: I pushed the updated file with some inline questions in a moment, would love to get your feedback there.

tasn assigned this task to woohyun.Oct 12 2019, 7:34 AM
zmike moved this task from Backlog to Evaluating on the efl: api board.Oct 14 2019, 5:56 AM