Dear All,

Hello!

I have attached patch for elm_diskselector.
This patch is setting the width of elm_diskselector using its parent width
size, if there is no mention in the theme file.


Sincerely,
Shinwoo Kim.



2011/4/7 Carsten Haitzler <ras...@rasterman.com>

> On Tue, 5 Apr 2011 23:33:08 +0900 cnook <kimci...@gmail.com> said:
>
> thanks. still for()s in the test code have no space between for and ( - i
> fixed
> it though. patch in svn now!
>
> > Hello!
> >
> > Thank you for your response.
> > I am thoroughly impressed with your in-depth review.
> > All of your feedback is reflected on the latest attached patch.
> > Please verify and review this again. Thanks!
> >
> >
> > Sincerely,
> > Shinwoo Kim.
> >
> > 2011/4/4 Carsten Haitzler <ras...@rasterman.com>
> >
> > > On Mon, 28 Mar 2011 12:04:19 +0900 cnook <kimci...@gmail.com> said:
> > >
> > > too many problems with this patch :( can you fix the below and
> re-submit?
> > >
> > > 1. fails. malformed patch even.
> > > patching file src/lib/elm_diskselector.c
> > > Hunk #5 FAILED at 136.
> > > Hunk #19 succeeded at 780 with fuzz 2.
> > > 1 out of 29 hunks FAILED -- saving rejects to file
> > > src/lib/elm_diskselector.c.rej patching file src/lib/Elementary.h.in
> > > patching file src/bin/test_diskselector.c
> > > patching file data/themes/default.edc
> > > patch: **** malformed patch at line 553:
> > > 2. math relying on operator precedence rather than being explicit with
> > > ()'s:
> > > +     evas_object_resize(wd->main_box, w / wd->display_item_num *
> > > (wd->item_count + CEIL(wd->display_item_num) * 2), h);
> > > +     evas_object_resize(wd->main_box, w / wd->display_item_num *
> > > (wd->item_count + CEIL(wd->display_item_num)), h);
> > > 3. divide by 0 waiting to happen:
> > > +                                 (int)(w / wd->display_item_num), 0);
> > > (you never check the return of atoi so it can happily return 0 or theme
> > > could
> > > literally use a 0)
> > > 4. no space between if and ():
> > > +   if(!wd->display_item_num_by_api)
> > > 5. no check of atoi result to stop invalid values (<= 0):
> > > +        if (str) wd->display_item_num = atoi(str);
> > > 6. no check of atoi() to check for <= -2:
> > > +   if (str) wd->minw = atoi(str);
> > > +   if (str) wd->minh = atoi(str);
> > > 7. no check of atoi result to stop invalid values:
> > > +   if (str) wd->display_item_num = atoi(str);
> > > 8. use: if (!strcmp(...)):
> > > +        if (strcmp(item->label, it->label) == 0)
> edje_object_signal_emit
> > > (item->base.view, "elm,state,selected", "elm");
> > > 9. add space between void and * and no space between ) and it->:
> > > +   if (it->func) it->func((void*) it->base.data, it->base.widget, it);
> > > 10. more operator precedence reliance here. use ()'s to collect
> statements:
> > > +        if (x > w / wd->display_item_num * (wd->item_count +
> > > (wd->display_item_num % 2)))
> > > +                                               x - w /
> > > wd->display_item_num *
> > > wd->item_count,
> > > +                                               x + w /
> > > wd->display_item_num *
> > > wd->item_count,
> > > 11. add space between for and (:
> > > +   for(i = 2; i < CEIL(wd->display_item_num); i++)
> > > +   for(i = 3; i <= CEIL(wd->display_item_num); i++)
> > > +             for(i = 2; i < CEIL(wd->display_item_num); i++)
> > > +             for(i = 3; i <= CEIL(wd->display_item_num); i++)
> > > +   for(idx = 0; idx < sizeof(month_list) / sizeof(month_list[0]);
> idx++)
> > > +   for(idx = 1; idx < 31; idx++)
> > > 12. please declare vars at top of function or {} section where you can:
> > > +   char *month_list[] = {
> > > +   char date[3];
> > >
> > >
> > > > Hi All,
> > > >
> > > > Thanks for your response and suggestion always.
> > > > I have attached new patch file which has following modification.
> > > >
> > > >   - remove warning message
> > > >   - support setting by theme and api
> > > >
> > > > Others are also reasonable suggestion, some of them especially
> vertical
> > > mode
> > > > would be fine. But till now there is no design for that as raster
> > > mentioned.
> > > > setting min/max "display item num" is reasonable also but user can
> set
> > > > content size differently. So if elm_diskselector restrict its min/max
> > > > "display item num", it would occur that user cannot set "display item
> > > num"
> > > > more than max value even though there is enough space.
> > > >
> > > > Thanks again.
> > > >
> > > > Sincerely,
> > > > Shinwoo Kim.
> > > >
> > > > 2011/3/25 Carsten Haitzler <ras...@rasterman.com>
> > > >
> > > > > On Wed, 23 Mar 2011 08:19:13 -0700 Daniel Juyung Seo <
> > > seojuyu...@gmail.com
> > > > > >
> > > > > said:
> > > > >
> > > > > you're going to hate me :)
> > > > >
> > > > > you did go the right way - theme defines default, code can
> override...
> > > > > BUT..
> > > > > you are ALSO missing a fit POLICY. you are fitting just N items in
> the
> > > > > visible
> > > > > region. i think... you are missing the ability to say "fit as many
> > > items
> > > > > ads
> > > > > you can" either based on largest item in list (homogeneous layout
> in
> > > the
> > > > > box)
> > > > > or have all items a different size (actually i might argue that
> this is
> > > > > dubiously useful except maybe in a case where MOST items are small
> and
> > > 1 or
> > > > > 2
> > > > > are longer eg a list like: 1,2,3,4,5,NONE).
> > > > >
> > > > > so THEME should provide the default FIT policy (fixed count, best
> fit,
> > > > > compact
> > > > > fit). fixed count uses the theme number defined as you did already
> and
> > > code
> > > > > can
> > > > > override that, and other policies as above. also code should be
> allowed
> > > to
> > > > > override this too.
> > > > >
> > > > > (and yes a horizontal scrolling list of items where they change
> size
> > > > > horizontally is not that great as gustavo mentioned... but i know
> that
> > > you
> > > > > didn't design the widget, so not a lot of use going on about that -
> BUT
> > > his
> > > > > point implies that we should have a vertical diskselector mode
> too...
> > > use
> > > > > that
> > > > > for horizontally expanding items, and vice-versa).
> > > > >
> > > > > > I attached a screen shot :)
> > > > > > This is a screen shot from elementary_test "Disk Selector" which
> > > cnook
> > > > > > attached.
> > > > > >
> > > > > > Btw, cnook, I have one comment.
> > > > > > _item_click_cb()'s parameter is wrong.
> > > > > > Please check elm_diskselector.c code.
> > > > > >
> > > > > > elm_diskselector.c: In function ‘_item_new’:
> > > > > > elm_diskselector.c:137: warning: passing argument 4 of
> > > > > > ‘edje_object_signal_callback_add’ f
> > > > > > rom incompatible pointer type
> > > > > > /usr/local/include/edje-1/Edje.h:550: note: expected
> ‘Edje_Signal_Cb’
> > > > > > but argument is of t
> > > > > > ype ‘void (*)(void *, struct Evas_Object *, void *)’
> > > > > >
> > > > > > Thanks.
> > > > > > Daniel Juyung Seo (SeoZ)
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Mar 23, 2011 at 7:15 AM, Gustavo Sverzut Barbieri
> > > > > > <barbi...@profusion.mobi> wrote:
> > > > > > > On Wed, Mar 23, 2011 at 1:54 PM, cnook <kimci...@gmail.com>
> wrote:
> > > > > > >> Dear All,
> > > > > > >>
> > > > > > >> Thanks for your response.
> > > > > > >>
> > > > > > >> I have attached "elm_diskselector" patch using API and getting
> > > default
> > > > > > >> value from theme.
> > > > > > >> But the default value from "elementary/themes/default.edc" is
> > > applied
> > > > > only.
> > > > > > >> If user wants new default value from his/her own theme file,
> it
> > > will
> > > > > not be
> > > > > > >> applied.
> > > > > > >>
> > > > > > >>
> > > > > > >> Why I have changed like this..  because there is one case I
> worry
> > > > > about
> > > > > > >
> > > > > > >
> > > > > > > Do you have the screenshot? Nobody replied to my mail.
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Gustavo Sverzut Barbieri
> > > > > > > http://profusion.mobi embedded systems
> > > > > > > --------------------------------------
> > > > > > > MSN: barbi...@gmail.com
> > > > > > > Skype: gsbarbieri
> > > > > > > Mobile: +55 (19) 9225-2202
> > > > > > >
> > > > > > >
> > > > >
> > >
> ------------------------------------------------------------------------------
> > > > > > > Enable your software for Intel(R) Active Management Technology
> to
> > > meet
> > > > > the
> > > > > > > growing manageability and security demands of your customers.
> > > > > Businesses
> > > > > > > are taking advantage of Intel(R) vPro (TM) technology - will
> your
> > > > > software
> > > > > > > be a part of the solution? Download the Intel(R) Manageability
> > > Checker
> > > > > > > today! http://p.sf.net/sfu/intel-dev2devmar
> > > > > > > _______________________________________________
> > > > > > > enlightenment-devel mailing list
> > > > > > > enlightenment-devel@lists.sourceforge.net
> > > > > > >
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> > > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > ------------- Codito, ergo sum - "I code, therefore I am"
> > > --------------
> > > > > The Rasterman (Carsten Haitzler)    ras...@rasterman.com
> > > > >
> > > > >
> > >
> > >
> > > --
> > > ------------- Codito, ergo sum - "I code, therefore I am"
> --------------
> > > The Rasterman (Carsten Haitzler)    ras...@rasterman.com
> > >
> > >
>
>
> --
> ------------- Codito, ergo sum - "I code, therefore I am" --------------
> The Rasterman (Carsten Haitzler)    ras...@rasterman.com
>
>
Index: src/lib/elm_diskselector.c
===================================================================
--- src/lib/elm_diskselector.c	(revision 58885)
+++ src/lib/elm_diskselector.c	(working copy)
@@ -168,6 +168,7 @@
 _theme_data_get(Widget_Data *wd)
 {
    const char* str;
+   Evas_Object *parent;
    str = edje_object_data_get(wd->right_blank, "len_threshold");
    if (str) wd->len_threshold = MAX(0, atoi(str));
    else wd->len_threshold = 0;
@@ -181,7 +182,12 @@
 
    str = edje_object_data_get(wd->right_blank, "min_width");
    if (str) wd->minw = MAX(-1, atoi(str));
-   else wd->minw = -1;
+   else
+     {
+        parent = elm_widget_parent_widget_get(wd->self);
+        if (!parent) wd->minw = -1;
+        else evas_object_geometry_get(parent, NULL, NULL, &wd->minw, NULL);
+     }
 
    str = edje_object_data_get(wd->right_blank, "min_height");
    if (str) wd->minh = MAX(-1, atoi(str));
------------------------------------------------------------------------------
Fulfilling the Lean Software Promise
Lean software platforms are now widely adopted and the benefits have been 
demonstrated beyond question. Learn why your peers are replacing JEE 
containers with lightweight application servers - and what you can gain 
from the move. http://p.sf.net/sfu/vmware-sfemails
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to