On Tue, 03 May 2011 14:50:02 +0300 Tom Hacohen <tom.haco...@partner.samsung.com> said:
> Ok, so you are not against moving the timer out of the edc and to elm, > right? You just want edje expose an API to set show_last or not, and > that elm will set it, and have the timer value set in a property of > textblock? correctnessipoos > -- > Tom. > > On Tue, 2011-05-03 at 19:58 +0900, Carsten Haitzler wrote: > > On Sun, 01 May 2011 10:33:37 +0300 Tom Hacohen > > <tom.haco...@partner.samsung.com> said: > > > > i'm not sure about this whole env var. here is why: > > > > 1. this should be elm config in the config struct (and file) for elm. sure, > > be able to OVERRIDE config with env var, but it should be there. why? this > > is the kind of feature that is specific to the "desktop vs > > vkbd/touchsrceen" world, and when you have a vkbd, you want to see the last > > char temporarily, but a real kbd you may not want to as you know for sure > > what you pressed, so this should be config there just like finger > > scrolling, select modes etc. > > 2. if we do #1 above this follows that somehow elm needs to apss this info > > to edje as to how to do this. sure - have edje do it, but elm is the > > controller. > > > > > Dear Shilpa, > > > > > > I vote for first, and get it from an env variable. Really > > > no reason why to involve Elementary in the process. > > > By not involving elementary you gain two major benefits. > > > 1. Less complication because there are less things to change (especially > > > because the change in ELM will have to be ugly and annoying). > > > 2. There's no reason why Edje only applications shouldn't benefit from > > > this change. > > > > > > -- > > > Tom. > > > > > > On Fri, 2011-04-29 at 03:49 +0000, SHILPAONKAR SINGH wrote: > > > > Hi Tom, > > > > > > > > > > > > > > > > Further patches will depend on the decision we take regarding timer > > > > > > > > once decided I will resend patches by splitting them. > > > > > > > > > > > > > > > > Dear All, > > > > > > > > Please let us know your opinion. > > > > > > > > 1. Create timer internally in edje, use a variable in theme to decide > > > > timer value, use a config variable of EDJE > > > > > > > > to decide the password mode[show last input/normal]. - simple and > > > > clear way. > > > > > > > > (or) > > > > > > > > 2. Use a config variable of elementary to decide password mode[show > > > > last/normal] , use a new theme for password-showlast > > > > > > > > mode. control timer[in and a new action] using a new theme. - A little > > > > complicated way which would avoid timer in edje. > > > > > > > > > > > > > > > > From My side I have already done second design, I will start working > > > > on first design as well. > > > > > > > > > > > > > > > > Thanks & Regards > > > > > > > > Shilpa Singh > > > > > > > > > > > > > > > > ------- Original Message ------- > > > > > > > > Sender : Tom Hacohen<tom.haco...@partner.samsung.com> > > > > Engineer/STRI-SLP RTL Language supporting/Samsung Electronics > > > > > > > > Date : Apr 28, 2011 23:28 (GMT+09:00) > > > > > > > > Title : Re: Re: Re: [E-devel] [Patch] Entry - Password show last input > > > > patch > > > > > > > > > > > > > > > > Dear Shilpa, > > > > > > > > Please start by splitting the patches and sending two different > > > > patches. > > > > It's really hard for me to further review the patches like this (that > > > > I > > > > don't know what I need to review and what not). > > > > > > > > Regarding what we discussed. Lets wait for others to reply, though > > > > honestly, I like setting time in theme and actually doing it in edje > > > > instead of setting a timer in the theme and implementing the timer > > > > there. > > > > > > > > -- > > > > Tom. > > > > > > > > On Thu, 2011-04-28 at 12:03 +0000, SHILPAONKAR SINGH wrote: > > > > > Dear Tom and Raster, > > > > > > > > > > Please find my reply inline in a more visible manner :). > > > > > > > > > > > > > > > > Please find my reply inline. > > > > > > 1. Looking at the changes you did for elementary: I don't see why > > > > > you > > > > > > even need them. Just change the env var to > > > > > EDJE_ENTRY_PASSWORD_SHOW_LAST > > > > > > and skip all the things you had to do in elm... Makes no sense to > > > > > have > > > > > > those in ELM. By doing that you get the added value of supporting > > > > > this > > > > > > feature in edje (without elm) which is wanted anyway. > > > > > > 2. Bullet #1 eliminates the need for > > > > > > "entry_mode: PASSWORD_SHOW_LAST_INPUT;" You just need to set it as > > > > > > password like it was, and merge the theme with the one of the > > > > > regular > > > > > > password. This will simplify some of your edje changes. > > > > > > Elementary: Thanks for your comments, we also had this > > > > > discussion long back :) > > > > > > 1,2. Q) why do we have 2 themes?: Ans: entry_mode: > > > > > PASSWORD_SHOW_LAST_INPUT is not the only difference, > > > > > > the major difference is in program section of these 2 themes. In > > > > the > > > > > showlastinput mode, two new programs have been > > > > > > added which are used to control the timer part. As you know the > > > > > password is visible for some time and is masked after some 2 > > > > seconds. > > > > > > a new action HIDE_VISIBLE_PASSWORD is created just for this > > > > purpose. > > > > > we use "in" to control the timer and when timer expires this action > > > > > > is triggered which calls a callback to remove the password tags. > > > > > [ This was Raster's Idea and when we had discussion, you also had > > > > > agreed with > > > > > > this idea :) ] By having a theme we can contol the timer value via > > > > > theme and eliminate the timer functionality from edje totally. > > > > Please > > > > > refer line numbers > > > > > > 143-154 in the elm_entry.patch. hence we use config variable in > > > > > elementary. > > > > > > > > > > Well, I'm not against controlling that in theme, I'm just wondering, > > > > > why > > > > > not support SHOW_LAST in edje as well? all the logic is done in edje > > > > > anyway (except for the timer), so why not remove the timer to inside > > > > > of > > > > > edje and just make that simpler without the need for another > > > > program? > > > > > (Like I suggested above). Maybe I've agreed with that in the past (I > > > > > honestly don't remember), but seeing it now it really looks odd and > > > > > complex for no reason.... We already have too many duplicated groups > > > > > in > > > > > ELM as it is, no need to dupe it more. > > > > > > > > > > We can just add a property in the theme that tells edje the delay > > > > time > > > > > (with edje having a default value of its own) which means it'll be > > > > > configurable by elementary and will just work for any edje app. This > > > > > will remove all this unneeded complexity and will make everything a > > > > > lot > > > > > clearer. I think this is the way to go, what do you think? > > > > > > > > > > (And create a timer in C inside the edje code instead of creating a > > > > > hackish timer in the theme...) > > > > > > > > > > The way suggested by you was the same way I was following before > > > > > My Old code created a ecore timer internally. Definitely this is a > > > > > more simple and understandable way. > > > > > but I had to change it as Raster wanted me to remove timer from edje > > > > > and do it via theme. > > > > > using HIDE_VISIBLE_PASSWORD action and "in". I think lets confirm > > > > > this > > > > > approach with Raster again before I send a new patch. please check > > > > the > > > > > chat below which we > > > > > had last time regarding this issue. > > > > > > > > > > > > > > > > > > > > Hi Raster, > > > > > > > > > > Kindly let me know your opinion. > > > > > > > > > > [ > > > > > > > in custom password two new programs have been added, these > > > > > programs > > > > > > > are used for the timer > > > > > > > > > > > > > > part[ using in and a new action HIDE_VISIBLE_PASSWORD is > > > > created > > > > > > > just for this purpose]. so As two themes > > > > > > > > > > > > > > are used two edit modes are used at edje level[ PASSWORD and > > > > > > > PASSWORD_SHOW_LAST_CHARACTER]. > > > > > > > > > > > > > > I did confirm the same with Raster before starting, > > > > > that > > > > > > > I have 2 themes for password mode at entry level and he > > > > > > > > > > > > > > said thats fine. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Raster what do you think about Tom's Idea? kindly let me know > > > > > your > > > > > > > opinion. > > > > > > Ok, saw raster's reply, so never mind this comment. > > > > > ] > > > > > > > > > > > > > > > > 3. I noticed you did a bunch of changes in edje (near preedit > > > > > mostly), > > > > > > what are they for? > > > > > > > > > > > > For Multitap Input 3x4 keyboard type and languages type of > > > > > input where we need to tap multiple > > > > > > times to arrive at a character - This fix is only for > > > > sofetkeyboard > > > > > i;e, ise. > > > > > > so you can check commit_cb and preedit_changed_cb, I add tag on > > > > > pre-edit and remove the tag on > > > > > > commit but if there is no pre-edit string then I directly add the > > > > > tag on commit. > > > > > > so until the user arrives at the required character in 3x4 or > > > > > language input user will be able to view the character > > > > > > once the character is committed it automatically turns in to *. > > > > > > The features have been tested with 3x4 input and language input on > > > > > the device. > > > > > > > > > > Well, those changes aren't really related to password mode, they are > > > > > just to fix bugs in preedit handling so please split them to another > > > > > patch. It's confusing to follow/review. > > > > > > > > > > I will do one thing I will only send the patch for keyboard and for > > > > > softkey board I will > > > > > send one more patch. > > > > > > > > > > > > > > > > 4. Regarding textblock: I understand why you did your change to > > > > > > evas_textblock_node_format_remove_pair, but the change isn't the > > > > > correct > > > > > > way to handle this. I'm currently working on fixing this function, > > > > > my > > > > > > bad for forgetting about it. :P > > > > > > When I called the function, though the tags were removed > > > > the > > > > > text > > > > > > was not immediately rendered hence I called the invalidate > > > > function > > > > > to > > > > > > render the text immediately. My requirement is that when I remove > > > > > the tag > > > > > > the display of text has to adjust immediately else the password > > > > > feature would be incomplete. > > > > > > > > > > Yes, I fixed that in upstream, would you mind checking? > > > > > > > > > > It works fine I updated and tested and Thanks for merging the patch. > > > > > > > > > > Thanks & Regards > > > > > Shilpa Singh > > > > > > > > > > > > > > > > > > > > ------- Original Message ------- > > > > > Sender : Tom Hacohen > > > > > Engineer/STRI-SLP RTL Language supporting/Samsung Electronics > > > > > Date : Apr 28, 2011 18:42 (GMT+09:00) > > > > > Title : Re: Re: [E-devel] [Patch] Entry - Password show last input > > > > > patch > > > > > > > > > > Dear Shilpa, > > > > > > > > > > Please mark your changes in a more visible way next time. :P I can't > > > > > understand who's who. > > > > > > > > > > My comments are also inlined. > > > > > On Thu, 2011-04-28 at 09:21 +0000, SHILPAONKAR SINGH wrote: > > > > > > Dear Tom, > > > > > > > > > > > > Please find my reply inline. > > > > > > 1. Looking at the changes you did for elementary: I don't see why > > > > > you > > > > > > even need them. Just change the env var to > > > > > EDJE_ENTRY_PASSWORD_SHOW_LAST > > > > > > and skip all the things you had to do in elm... Makes no sense to > > > > > have > > > > > > those in ELM. By doing that you get the added value of supporting > > > > > this > > > > > > feature in edje (without elm) which is wanted anyway. > > > > > > 2. Bullet #1 eliminates the need for > > > > > > "entry_mode: PASSWORD_SHOW_LAST_INPUT;" You just need to set it as > > > > > > password like it was, and merge the theme with the one of the > > > > > regular > > > > > > password. This will simplify some of your edje changes. > > > > > > Elementary: Thanks for your comments, we also had this > > > > > discussion long back :) > > > > > > 1,2. Q) why do we have 2 themes?: Ans: entry_mode: > > > > > PASSWORD_SHOW_LAST_INPUT is not the only difference, > > > > > > the major difference is in program section of these 2 themes. In > > > > the > > > > > showlastinput mode, two new programs have been > > > > > > added which are used to control the timer part. As you know the > > > > > password is visible for some time and is masked after some 2 > > > > seconds. > > > > > > a new action HIDE_VISIBLE_PASSWORD is created just for this > > > > purpose. > > > > > we use "in" to control the timer and when timer expires this action > > > > > > is triggered which calls a callback to remove the password tags. > > > > > [ This was Raster's Idea and when we had discussion, you also had > > > > > agreed with > > > > > > this idea :) ] By having a theme we can contol the timer value via > > > > > theme and eliminate the timer functionality from edje totally. > > > > Please > > > > > refer line numbers > > > > > > 143-154 in the elm_entry.patch. hence we use config variable in > > > > > elementary. > > > > > > > > > > Well, I'm not against controlling that in theme, I'm just wondering, > > > > > why > > > > > not support SHOW_LAST in edje as well? all the logic is done in edje > > > > > anyway (except for the timer), so why not remove the timer to inside > > > > > of > > > > > edje and just make that simpler without the need for another > > > > program? > > > > > (Like I suggested above). Maybe I've agreed with that in the past (I > > > > > honestly don't remember), but seeing it now it really looks odd and > > > > > complex for no reason.... We already have too many duplicated groups > > > > > in > > > > > ELM as it is, no need to dupe it more. > > > > > > > > > > We can just add a property in the theme that tells edje the delay > > > > time > > > > > (with edje having a default value of its own) which means it'll be > > > > > configurable by elementary and will just work for any edje app. This > > > > > will remove all this unneeded complexity and will make everything a > > > > > lot > > > > > clearer. I think this is the way to go, what do you think? > > > > > > > > > > (And create a timer in C inside the edje code instead of creating a > > > > > hackish timer in the theme...) > > > > > > > > > > -- > > > > > Tom. > > > > > > > > > > > > 3. I noticed you did a bunch of changes in edje (near preedit > > > > > mostly), > > > > > > what are they for? > > > > > > > > > > > > For Multitap Input 3x4 keyboard type and languages type of > > > > > input where we need to tap multiple > > > > > > times to arrive at a character - This fix is only for > > > > sofetkeyboard > > > > > i;e, ise. > > > > > > so you can check commit_cb and preedit_changed_cb, I add tag on > > > > > pre-edit and remove the tag on > > > > > > commit but if there is no pre-edit string then I directly add the > > > > > tag on commit. > > > > > > so until the user arrives at the required character in 3x4 or > > > > > language input user will be able to view the character > > > > > > once the character is committed it automatically turns in to *. > > > > > > The features have been tested with 3x4 input and language input on > > > > > the device. > > > > > > > > > > Well, those changes aren't really related to password mode, they are > > > > > just to fix bugs in preedit handling so please split them to another > > > > > patch. It's confusing to follow/review. > > > > > > > > > > > > 4. Regarding textblock: I understand why you did your change to > > > > > > evas_textblock_node_format_remove_pair, but the change isn't the > > > > > correct > > > > > > way to handle this. I'm currently working on fixing this function, > > > > > my > > > > > > bad for forgetting about it. :P > > > > > > When I called the function, though the tags were removed > > > > the > > > > > text > > > > > > was not immediately rendered hence I called the invalidate > > > > function > > > > > to > > > > > > render the text immediately. My requirement is that when I remove > > > > > the tag > > > > > > the display of text has to adjust immediately else the password > > > > > feature would be incomplete. > > > > > > > > > > Yes, I fixed that in upstream, would you mind checking? > > > > > > > > > > > > Thanks & Regards > > > > > > Shilpa Singh > > > > > > > > > > > > ------- Original Message ------- > > > > > > Sender : Tom Hacohen Engineer/STRI-SLP RTL Language > > > > > supporting/Samsung Electronics > > > > > > Date : Apr 28, 2011 16:58 (GMT+09:00) > > > > > > Title : Re: [E-devel] [Patch] Entry - Password show last input > > > > patch > > > > > > > > > > > > Dear Shilpa, > > > > > > > > > > > > Here are my comments: > > > > > > 1. Looking at the changes you did for elementary: I don't see why > > > > > you > > > > > > even need them. Just change the env var to > > > > > EDJE_ENTRY_PASSWORD_SHOW_LAST > > > > > > and skip all the things you had to do in elm... Makes no sense to > > > > > have > > > > > > those in ELM. By doing that you get the added value of supporting > > > > > this > > > > > > feature in edje (without elm) which is wanted anyway. > > > > > > 2. Bullet #1 eliminates the need for > > > > > > "entry_mode: PASSWORD_SHOW_LAST_INPUT;" You just need to set it as > > > > > > password like it was, and merge the theme with the one of the > > > > > regular > > > > > > password. This will simplify some of your edje changes. > > > > > > 3. I noticed you did a bunch of changes in edje (near preedit > > > > > mostly), > > > > > > what are they for? > > > > > > 4. Regarding textblock: I understand why you did your change to > > > > > > evas_textblock_node_format_remove_pair, but the change isn't the > > > > > correct > > > > > > way to handle this. I'm currently working on fixing this function, > > > > > my > > > > > > bad for forgetting about it. :P > > > > > > > > > > > > Please fix/explain the above comments and then I will review the > > > > > patch > > > > > > again. > > > > > > > > > > > > Thanks, > > > > > > Tom. > > > > > > > > > > > > > > > > > > On Thu, 2011-04-28 at 14:38 +0900, Daniel Juyung Seo wrote: > > > > > > > Dear all, I forward Shilpaonkar Singh's entry patch to mailing > > > > > list. > > > > > > > Please find the attached mail and patch files. > > > > > > > > > > > > > > > > > > > > > ------- Original Message ------- > > > > > > > > > > > > > > Sender : SHILPAONKAR SINGH > > > > > > > Date : Apr 27, 2011 22:22 (GMT+09:00) > > > > > > > Title : Entry - [Password show last input] open source patch > > > > > > > > > > > > > > Hi Mr. Seo, > > > > > > > > > > > > > > Please find attached patches for last input show password > > > > feature. > > > > > > > These patches are based on current SVN version - 58957. > > > > > > > > > > > > > > There is no special demo app for this feature, for testing > > > > purpose > > > > > > > enviromental variable > > > > > > > ELM_PASSWORD_SHOW_LAST_INPUT has to be set as shown below. > > > > > > > export ELM_PASSWORD_SHOW_LAST_INPUT = 1 and then by clicking on > > > > > > > entry_scrolled > > > > > > > we can test password feature. > > > > > > > > > > > > > > I have also done a fix in evas_textblock_node_format_remove_pair > > > > > function > > > > > > > for this feature in the current svn version. > > > > > > > Can you please push this patch to community as there are alot of > > > > > changes > > > > > > > happening in textblock level it would be better > > > > > > > to push it soon to avoid rework. > > > > > > > > > > > > > > Thanks & Regards > > > > > > > > > > > > > > Shilpa Singh > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------------------ > > > > > > > WhatsUp Gold - Download Free Network Management Software > > > > > > > The most intuitive, comprehensive, and cost-effective network > > > > > > > management toolset available today. Delivers lowest initial > > > > > > > acquisition cost and overall TCO of any competing solution. > > > > > > > http://p.sf.net/sfu/whatsupgold-sd > > > > > > > _______________________________________________ > > > > > > > enlightenment-devel mailing list > > > > > > > enlightenment-devel@lists.sourceforge.net > > > > > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks & Regards > > > > > > shilpa singh > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks & Regards > > > > > shilpa singh > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks & Regards > > > > > > > > shilpa singh > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------------------ > > > WhatsUp Gold - Download Free Network Management Software > > > The most intuitive, comprehensive, and cost-effective network > > > management toolset available today. Delivers lowest initial > > > acquisition cost and overall TCO of any competing solution. > > > http://p.sf.net/sfu/whatsupgold-sd > > > _______________________________________________ > > > 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 ------------------------------------------------------------------------------ WhatsUp Gold - Download Free Network Management Software The most intuitive, comprehensive, and cost-effective network management toolset available today. Delivers lowest initial acquisition cost and overall TCO of any competing solution. http://p.sf.net/sfu/whatsupgold-sd _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
