Page MenuHomePhabricator

GUI: Introduce a feature to selectively generate applications
Needs ReviewPublic

Authored by akanad on Dec 27 2018, 12:06 AM.

Details

Summary

this patch introduces a feature to selectively generate applications from db.

once user generates a database,
only contained widgets in a certain database will be shown in the checkbox list.

applications generated with limited widgets from database by selecting the checkboxes,
will contain the selected widgets only.

Signed-off-by: Wonki Kim <wonki_.kim@samsung.com>

Diff Detail

Branch
arcpatch-D7520
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8660
Build 7682: arc lint + arc unit
akanad requested review of this revision.Dec 27 2018, 12:06 AM
akanad created this revision.

Hi Wonki,

I began to review and comment the code but I think the design should be modified. As I described in the first comment, the type/alias info should be specified in the class JSON info itself, as we already gather DB information there. Btw, alias is imo wrongly used here. For example, Box is the alias for Elm.Box and Efl.Ui.Box, not the opposite.
I don't understand the "Select All Containers and Widgets" check box. Should not it be the same as "Use the entire Database"? The wdgs that can be generated must belong to the DB anyway, no?

By reading your code, I though about that design:

  • JSON info is stored in the class info: "type": "Containers", "alias": "Box"
  • db_containing_... should be renamed as db_classes_get and return the list of classes names. Walk on the list and
  • You don't need the _Check_Info structure
    • alias and type are only needed when the panels are created
    • widget_name and should_enabled are useless
    • is_checked can be stored in _Widget as is_gen_enabled. It should be set to true when a DB is loaded and when the check boxes are changed (via an API db_class_allow(db, kl_name, flag) or something like that). The check callbacks should invoke set/unset this flag.
    • Eo * is not needed
  • When DB is activated:
    • Clear the check boxes panel
    • Retrieve the DB class names via db_classes_get
    • Find in JSON the alias and the type; create the groups and the check boxes accordingly. The check callback has as data the list of the class names. When the callback is called, walk over the classes and invoke db_class_allow.
  • When the ML is generated, you just have to check if _Widget->is_gen_enabled is true

What do you think?

data/config.json
2

How do you differentiate between the widgets? I mean, how do you know that Table is a Container and not a simple Widget?
In the way you wrote it, you can add widgets to the list only my mentioning them inside this structure. What I suggested once is to insert that data inside Class_Properties->$class->DB->Type: "Container". You can even add an alias key "Box".
In this way, after you opened the DB, you retrieve the used classes and display them in their dedicated groups, depending on the type. If no type is given, you can by default display them in "Widget". The advantage is that you see easily if a type misses for a widget by checking the Widget list in the GUI.
The other advantage would be that the groups constructions is not static (except Widgets = Default) but depends on the widgets types that you have in your DB. It means that you can extract the types from the config.json file and build the groups list with that.

src/bin/db.c
3297

You should check the unicity

src/bin/db.h
21

Why not db_classes_get... that return a list of class names?
containing widgets seem to be the all the widgets info.

akanad added a comment.Jan 3 2019, 9:51 PM

I thought that I should abstract classes so that user don't need to know about a detail of the classes.
I mean I guess that some user doesn't really need to know about what kind of actual classes for "Box" are there.
if they select "Box" then ea_gen would provide a kind of boxes such as elm.box or efl.ui.box.
I agree that "alias" is not perfectly suitable here. so that how about changing it to something like "actual_classes"?

data/config.json
2

Widget_Type on line #2 is not referred from anywhere.
I thought that I would add the information to differentiate a type of components while scratching. (the line has been added by mistake. my bad.. ) however I don't think It really matters what a type of some components is. but I guess that it matters what components would a user select to generate applications.

src/bin/db.c
3297

I do not get what you said about unicity. this comment is for telling about function name?
if not, would you please explain more detail ?

src/bin/db.h
21

I will do that.

akanad updated this revision to Diff 18214.Jan 3 2019, 9:54 PM

changes the name of a property for config.json
changes the name of a function for db
changes misc for gui

JackDanielZ added inline comments.Jan 5 2019, 11:01 AM
data/config.json
2

Sorry but I am confused. Don't you want to differentiate widgets that are objects containers vs items containers vs basic widgets vs... ?

"My" type would be added to the JSON widget description with the alias.

The way you wrote the JSON part forces you to a subset of the widgets. It means that every time a widget will be added, you will have to add it to that name, since you show the check boxes based on that JSON list. It means you increase maintenance.

src/bin/db.c
3297

If you have 2000 genlist items in the ML file, you will add the class name "Elm.Genlist.Item" 2000 times instead of just one

akanad added inline comments.Jan 9 2019, 12:58 AM
data/config.json
2

I am confused too.
even if we decided to add widget description with the alias,
thing that someone should add widget information once new widgets are added,
is not changed. isn't it?

src/bin/db.c
3297

would I ? I guess that wdgs list has a single Elm.Genlist.Item because it's constructed by hashing.
I'm saying while seeing _wdg_get_by_kl function.