[PATCH] toaster: enable add dependent layer button

[PATCH] toaster: enable add dependent layer button

[Reyna, David]

[-- Attachment #1: Type: text/plain, Size: 2787 bytes --]

Hi all,

I have posted a patch for 9936 here:
   http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib?h=dreyna%2Flayer_dep_button_9936

The patch content is below. Note that I had to do trial and error to find the actual arguments sent to the "typeahead:render" event handler, because it does not match the documentation, and I have included a comment in the patch to pass that information on. The "typeahead:render" event handler also often gets intermediate calls with no items in its parameter list, so those have to be ignored.

I did a git send mail, but I have not seen it reflect yet from the Toaster email list.

- David

==============
From cc2a697632e6101aff17973cb7899edbbe8e5ad4 Mon Sep 17 00:00:00 2001
From: David Reyna <David.Reyna@windriver.com>
Date: Tue, 23 Aug 2016 16:40:58 -0700
Subject: toaster: enable add dependent layer button

The "typeahead:select" event is added to enable the add
dependent layer button when a layer is selected from the
type ahead list.

The "typeahead:render" event is added to fill a list with
the current filtered type ahead list of matching layer
names, so that the "input change" event knows to enable
the import layer button if the layer name is in that
captured list.

[YOCTO #9936]

Signed-off-by: David Reyna <david.reyna@windriver.com>

diff --git a/bitbake/lib/toaster/toastergui/static/js/importlayer.js b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
index 191b30f..08142e8 100644
--- a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
+++ b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
@@ -24,15 +24,23 @@ function importLayerPageInit (ctx) {
   // choices available in the typeahead
   var layerDepsChoices = {};

+  // when a typeahead choice is selected, enabling the "Add layer" button
+  // since we know for sure this is a valid layer
+  layerDepInput.on("typeahead:select", function (event, data) {
+    if (data.name) {
+      layerDepBtn.removeAttr("disabled");
+    }
+  });
+
   // when the typeahead choices change, store an array of the available layer
   // choices locally, to use for enabling/disabling the "Add layer" button
-  layerDepInput.on("typeahead-choices-change", function (event, data) {
-    layerDepsChoices = {};
-
-    if (data.choices) {
-      data.choices.forEach(function (item) {
-        layerDepsChoices[item.name] = item;
-      });
+  //  layerDepInput.on("typeahead:render", function (event, [item[,item]*]) {
+  layerDepInput.on("typeahead:render", function (event, items) {
+    for (var i = 1, j = arguments.length; i < j; i++){
+      if (i == 1) {
+        layerDepsChoices = {};
+      }
+      layerDepsChoices[arguments[i].name] = arguments[i];
     }
   });

--
cgit v0.10.2



[-- Attachment #2: Type: text/html, Size: 9175 bytes --]

Re: [PATCH] toaster: enable add dependent layer button

[Barros Pena, Belen]

On 24/08/2016 09:25, "Reyna, David" <david.reyna@windriver.com> wrote:

>Hi all,
> 
>I have posted a patch for 9936 here:
>   
>http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib?h=dreyna%2Flayer_de
>p_button_9936 
><http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib?h=dreyna%2Flayer_d
>ep_button_9936>

From the UI side, this looks good to me.

Thanks!

Belén

> 
>The patch content is below. Note that I had to do trial and error to find
>the actual arguments sent to the
>"typeahead:render" event handler, because it does not match the
>documentation, and I have included
> a comment in the patch
>to pass that information on. The
>"typeahead:render" event handler also
>often gets intermediate calls
> with no items in its parameter list, so those have to be ignored.
> 
>I did a git send mail, but I have not seen it reflect yet from the
>Toaster email list.
> 
>- David
> 
>==============
From cc2a697632e6101aff17973cb7899edbbe8e5ad4 Mon Sep 17 00:00:00 2001
>From: David Reyna <David.Reyna@windriver.com>
>Date: Tue, 23 Aug 2016 16:40:58 -0700
>Subject: toaster: enable add dependent layer button
> 
>The "typeahead:select" event is added to enable the add
>dependent layer button when a layer is selected from the
>type ahead list.
> 
>The "typeahead:render" event is added to fill a list with
>the current filtered type ahead list of matching layer
>names, so that the "input change" event knows to enable
>the import layer button if the layer name is in that
>captured list.
> 
>[YOCTO #9936]
> 
>Signed-off-by: David Reyna <david.reyna@windriver.com>
> 
>diff --git a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
>b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
>index 191b30f..08142e8 100644
>--- a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
>+++ b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
>@@ -24,15 +24,23 @@ function importLayerPageInit (ctx) {
>   // choices available in the typeahead
>   var layerDepsChoices = {};
> 
>+  // when a typeahead choice is selected, enabling the "Add layer" button
>+  // since we know for sure this is a valid layer
>+  layerDepInput.on("typeahead:select", function (event, data) {
>+    if (data.name) {
>+      layerDepBtn.removeAttr("disabled");
>+    }
>+  });
>+
>   // when the typeahead choices change, store an array of the available
>layer
>   // choices locally, to use for enabling/disabling the "Add layer"
>button
>-  layerDepInput.on("typeahead-choices-change", function (event, data) {
>-    layerDepsChoices = {};
>-
>-    if (data.choices) {
>-      data.choices.forEach(function (item) {
>-        layerDepsChoices[item.name] = item;
>-      });
>+  //  layerDepInput.on("typeahead:render", function (event,
>[item[,item]*]) {
>+  layerDepInput.on("typeahead:render", function (event, items) {
>+    for (var i = 1, j = arguments.length; i < j; i++){
>+      if (i == 1) {
>+        layerDepsChoices = {};
>+      }
>+      layerDepsChoices[arguments[i].name] = arguments[i];
>     }
>   });
> 
>-- 
>cgit v0.10.2
> 
> 

Re: [PATCH] toaster: enable add dependent layer button

[sujith h]

[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]

Hi,

Overall the patch looks good. Only small suggestion that came to my mind is
mentioned below, inline.

On Wed, Aug 24, 2016 at 1:55 PM, Reyna, David <david.reyna@windriver.com>
wrote:

> Hi all,
>
> I have posted a patch for 9936 here:
>    http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib?h=
> dreyna%2Flayer_dep_button_9936
>
> The patch content is below. Note that I had to do trial and error to find
> the actual arguments sent to the "typeahead:render" event handler,
> because it does not match the documentation, and I have included a
> comment in the patch to pass that information on. The "typeahead:render"
> event handler also often gets intermediate calls with no items in its
> parameter list, so those have to be ignored.
>
> I did a git send mail, but I have not seen it reflect yet from the Toaster
> email list.
>
> - David
>
> ==============
> From cc2a697632e6101aff17973cb7899edbbe8e5ad4 Mon Sep 17 00:00:00 2001
> From: David Reyna <David.Reyna@windriver.com>
> Date: Tue, 23 Aug 2016 16:40:58 -0700
> Subject: toaster: enable add dependent layer button
>
> The "typeahead:select" event is added to enable the add
> dependent layer button when a layer is selected from the
> type ahead list.
>
> The "typeahead:render" event is added to fill a list with
> the current filtered type ahead list of matching layer
> names, so that the "input change" event knows to enable
> the import layer button if the layer name is in that
> captured list.
>
> [YOCTO #9936]
>
> Signed-off-by: David Reyna <david.reyna@windriver.com>
>
> diff --git a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
> b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
> index 191b30f..08142e8 100644
> --- a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
> +++ b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
> @@ -24,15 +24,23 @@ function importLayerPageInit (ctx) {
>    // choices available in the typeahead
>    var layerDepsChoices = {};
>
> +  // when a typeahead choice is selected, enabling the "Add layer" button
> +  // since we know for sure this is a valid layer
> +  layerDepInput.on("typeahead:select", function (event, data) {
> +    if (data.name) {
> +      layerDepBtn.removeAttr("disabled");
> +    }
> +  });
> +
>    // when the typeahead choices change, store an array of the available
> layer
>    // choices locally, to use for enabling/disabling the "Add layer" button
> -  layerDepInput.on("typeahead-choices-change", function (event, data) {
> -    layerDepsChoices = {};
> -
> -    if (data.choices) {
> -      data.choices.forEach(function (item) {
> -        layerDepsChoices[item.name] = item;
> -      });
> +  //  layerDepInput.on("typeahead:render", function (event,
> [item[,item]*]) {
> +  layerDepInput.on("typeahead:render", function (event, items) {
> +    for (var i = 1, j = arguments.length; i < j; i++){
> +      if (i == 1) {
> +        layerDepsChoices = {};
> +      }
>

Instead of emptying layerDepsChoices here, wouldn't it be better to move
layerDepsChoices = {}; line to after the for loop line? That way we can
remove the if condition too.

+      layerDepsChoices[arguments[i].name] = arguments[i];
>      }
>    });
>
> --
> cgit v0.10.2
>
>
>
> --
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>
>


-- 
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
<Project>Contributor to Yocto project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info
C-x C-c

[-- Attachment #2: Type: text/html, Size: 10488 bytes --]

Re: [PATCH] toaster: enable add dependent layer button

[Reyna, David]

[-- Attachment #1: Type: text/plain, Size: 5011 bytes --]

Hi Sujith,

Thank you for looking at my patch. I was wondering when it would get moved forward. Belen had already approved it at the UI level.

> “Instead of emptying layerDepsChoices here, wouldn't it be better to move layerDepsChoices = {}; line to after the for loop line? That way we can remove the if condition too. “
The processing timing is tricky. It turns out that this code path gets called multiple times with different states when the page is refreshing - I think that there is for example a fade-in loop happening. When I had the clear outside the loop I kept getting it emptied, then filled and then emptied again before it was displayed - even though there was valid data - which was quite annoying.
I finally wrote the code to only pre-clear the list when (a) there are any new entries and the outer loop is entered at all, and (b) right before the first item add. While it looks odd, it is safe and sane compared to the multiple re-draw calls, and this is the version that worked for all the valid and invalid cases I could come up with.
- David


From: sujith h [mailto:sujith.h@gmail.com]
Sent: Wednesday, September 28, 2016 11:39 PM
To: Reyna, David
Cc: toaster@yoctoproject.org; WOOD, MICHAEL
Subject: Re: [Toaster] [PATCH] toaster: enable add dependent layer button

Hi,
Overall the patch looks good. Only small suggestion that came to my mind is mentioned below, inline.

On Wed, Aug 24, 2016 at 1:55 PM, Reyna, David <david.reyna@windriver.com<mailto:david.reyna@windriver.com>> wrote:
Hi all,

I have posted a patch for 9936 here:
   http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib?h=dreyna%2Flayer_dep_button_9936

The patch content is below. Note that I had to do trial and error to find the actual arguments sent to the "typeahead:render" event handler, because it does not match the documentation, and I have included a comment in the patch to pass that information on. The "typeahead:render" event handler also often gets intermediate calls with no items in its parameter list, so those have to be ignored.

I did a git send mail, but I have not seen it reflect yet from the Toaster email list.

- David

==============
From cc2a697632e6101aff17973cb7899edbbe8e5ad4 Mon Sep 17 00:00:00 2001
From: David Reyna <David.Reyna@windriver.com<mailto:David.Reyna@windriver.com>>
Date: Tue, 23 Aug 2016 16:40:58 -0700
Subject: toaster: enable add dependent layer button

The "typeahead:select" event is added to enable the add
dependent layer button when a layer is selected from the
type ahead list.

The "typeahead:render" event is added to fill a list with
the current filtered type ahead list of matching layer
names, so that the "input change" event knows to enable
the import layer button if the layer name is in that
captured list.

[YOCTO #9936]

Signed-off-by: David Reyna <david.reyna@windriver.com<mailto:david.reyna@windriver.com>>

diff --git a/bitbake/lib/toaster/toastergui/static/js/importlayer.js b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
index 191b30f..08142e8 100644
--- a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
+++ b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
@@ -24,15 +24,23 @@ function importLayerPageInit (ctx) {
   // choices available in the typeahead
   var layerDepsChoices = {};

+  // when a typeahead choice is selected, enabling the "Add layer" button
+  // since we know for sure this is a valid layer
+  layerDepInput.on("typeahead:select", function (event, data) {
+    if (data.name<http://data.name>) {
+      layerDepBtn.removeAttr("disabled");
+    }
+  });
+
   // when the typeahead choices change, store an array of the available layer
   // choices locally, to use for enabling/disabling the "Add layer" button
-  layerDepInput.on("typeahead-choices-change", function (event, data) {
-    layerDepsChoices = {};
-
-    if (data.choices) {
-      data.choices.forEach(function (item) {
-        layerDepsChoices[item.name<http://item.name>] = item;
-      });
+  //  layerDepInput.on("typeahead:render", function (event, [item[,item]*]) {
+  layerDepInput.on("typeahead:render", function (event, items) {
+    for (var i = 1, j = arguments.length; i < j; i++){
+      if (i == 1) {
+        layerDepsChoices = {};
+      }

Instead of emptying layerDepsChoices here, wouldn't it be better to move layerDepsChoices = {}; line to after the for loop line? That way we can remove the if condition too.
+      layerDepsChoices[arguments[i].name] = arguments[i];
     }
   });

--
cgit v0.10.2



--
_______________________________________________
toaster mailing list
toaster@yoctoproject.org<mailto:toaster@yoctoproject.org>
https://lists.yoctoproject.org/listinfo/toaster



--
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
<Project>Contributor to Yocto project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info
C-x C-c

[-- Attachment #2: Type: text/html, Size: 24439 bytes --]

Re: [PATCH] toaster: enable add dependent layer button

[sujith h]

[-- Attachment #1: Type: text/plain, Size: 5758 bytes --]

On Fri, Sep 30, 2016 at 11:06 AM, Reyna, David <david.reyna@windriver.com>
wrote:

> Hi Sujith,
>
>
>
> Thank you for looking at my patch. I was wondering when it would get moved
> forward. Belen had already approved it at the UI level.
>
>
>
> > “Instead of emptying layerDepsChoices here, wouldn't it be better to
> move layerDepsChoices = {}; line to after the for loop line? That way we
> can remove the if condition too. “
>
> The processing timing is tricky. It turns out that this code path gets
> called multiple times with different states when the page is refreshing - I
> think that there is for example a fade-in loop happening. When I had the
> clear outside the loop I kept getting it emptied, then filled and then
> emptied again before it was displayed - even though there was valid data -
> which was quite annoying.
>
> I finally wrote the code to only pre-clear the list when (a) there are any
> new entries and the outer loop is entered at all, and (b) right before the
> first item add. While it looks odd, it is safe and sane compared to the
> multiple re-draw calls, and this is the version that worked for all the
> valid and invalid cases I could come up with.
>
Ok. No problem. It sounds reasonable :)

> - David
>
>
>
>
>
> *From:* sujith h [mailto:sujith.h@gmail.com]
> *Sent:* Wednesday, September 28, 2016 11:39 PM
> *To:* Reyna, David
> *Cc:* toaster@yoctoproject.org; WOOD, MICHAEL
> *Subject:* Re: [Toaster] [PATCH] toaster: enable add dependent layer
> button
>
>
>
> Hi,
>
> Overall the patch looks good. Only small suggestion that came to my mind
> is mentioned below, inline.
>
>
>
> On Wed, Aug 24, 2016 at 1:55 PM, Reyna, David <david.reyna@windriver.com>
> wrote:
>
> Hi all,
>
>
>
> I have posted a patch for 9936 here:
>
>    http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib?h=
> dreyna%2Flayer_dep_button_9936
>
>
>
> The patch content is below. Note that I had to do trial and error to find
> the actual arguments sent to the "typeahead:render" event handler,
> because it does not match the documentation, and I have included a comment
> in the patch to pass that information on. The "typeahead:render" event
> handler also often gets intermediate calls with no items in its parameter
> list, so those have to be ignored.
>
>
>
> I did a git send mail, but I have not seen it reflect yet from the Toaster
> email list.
>
>
>
> - David
>
>
>
> ==============
>
> From cc2a697632e6101aff17973cb7899edbbe8e5ad4 Mon Sep 17 00:00:00 2001
>
> From: David Reyna <David.Reyna@windriver.com>
>
> Date: Tue, 23 Aug 2016 16:40:58 -0700
>
> Subject: toaster: enable add dependent layer button
>
>
>
> The "typeahead:select" event is added to enable the add
>
> dependent layer button when a layer is selected from the
>
> type ahead list.
>
>
>
> The "typeahead:render" event is added to fill a list with
>
> the current filtered type ahead list of matching layer
>
> names, so that the "input change" event knows to enable
>
> the import layer button if the layer name is in that
>
> captured list.
>
>
>
> [YOCTO #9936]
>
>
>
> Signed-off-by: David Reyna <david.reyna@windriver.com>
>
>
>
> diff --git a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
> b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
>
> index 191b30f..08142e8 100644
>
> --- a/bitbake/lib/toaster/toastergui/static/js/importlayer.js
>
> +++ b/bitbake/lib/toaster/toastergui/static/js/importlayer.js
>
> @@ -24,15 +24,23 @@ function importLayerPageInit (ctx) {
>
>    // choices available in the typeahead
>
>    var layerDepsChoices = {};
>
>
>
> +  // when a typeahead choice is selected, enabling the "Add layer" button
>
> +  // since we know for sure this is a valid layer
>
> +  layerDepInput.on("typeahead:select", function (event, data) {
>
> +    if (data.name) {
>
> +      layerDepBtn.removeAttr("disabled");
>
> +    }
>
> +  });
>
> +
>
>    // when the typeahead choices change, store an array of the available
> layer
>
>    // choices locally, to use for enabling/disabling the "Add layer" button
>
> -  layerDepInput.on("typeahead-choices-change", function (event, data) {
>
> -    layerDepsChoices = {};
>
> -
>
> -    if (data.choices) {
>
> -      data.choices.forEach(function (item) {
>
> -        layerDepsChoices[item.name] = item;
>
> -      });
>
> +  //  layerDepInput.on("typeahead:render", function (event,
> [item[,item]*]) {
>
> +  layerDepInput.on("typeahead:render", function (event, items) {
>
> +    for (var i = 1, j = arguments.length; i < j; i++){
>
> +      if (i == 1) {
>
> +        layerDepsChoices = {};
>
> +      }
>
>
> Instead of emptying layerDepsChoices here, wouldn't it be better to move
> layerDepsChoices = {}; line to after the for loop line? That way we can
> remove the if condition too.
>
> +      layerDepsChoices[arguments[i].name] = arguments[i];
>
>      }
>
>    });
>
>
>
> --
>
> cgit v0.10.2
>
>
>
>
>
>
> --
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>
>
>
>
> --
>
> സുജിത് ഹരിദാസന്
> Bangalore
> <Project>Contributor to KDE project
>
> <Project>Contributor to Yocto project
>
> http://fci.wikia.com/wiki/Anti-DRM-Campaign
> <Blog> http://sujithh.info
>
> C-x C-c
>



-- 
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
<Project>Contributor to Yocto project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info
C-x C-c

[-- Attachment #2: Type: text/html, Size: 23910 bytes --]