All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
@ 2015-09-02  7:44 Reyna, David
  2015-09-02  9:16 ` Barros Pena, Belen
  0 siblings, 1 reply; 12+ messages in thread
From: Reyna, David @ 2015-09-02  7:44 UTC (permalink / raw)
  To: BARROS PENA, BELEN; +Cc: toaster@yoctoproject.org

Hi Belén,

Please find the patch for 8126 here:

   dreyna/project_fstypes_8126

Note: for the message 'label' the I insert and then show when there are no matches, it is guaranteed not to pollute the database because it can never be in the checked state.

- David


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-02  7:44 [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited Reyna, David
@ 2015-09-02  9:16 ` Barros Pena, Belen
  2015-09-02 14:46   ` Reyna, David
  0 siblings, 1 reply; 12+ messages in thread
From: Barros Pena, Belen @ 2015-09-02  9:16 UTC (permalink / raw)
  To: Reyna, David L (Wind River); +Cc: toaster@yoctoproject.org



On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com> wrote:

>Hi Belén,
>
>Please find the patch for 8126 here:
>
>   dreyna/project_fstypes_8126

Hi David,

This is looking fairly good. I've only run across one problem. This is how
to reproduce:

1. Click the 'change' icon for IMAGE_FSTYPES

2. Deselect all values: the 'save' button becomes disabled and the message
asking you to select at least one image type appears. This is the expected
behaviour

3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the way it
was before you clicked the 'change' icon. This is once more the correct
behaviour

4. Now click the 'change' icon again. There are image types selected, but
the message 'You must select at least one image type' still shows, and the
'Save' button is disabled. This is not the correct behaviour. As long as
there is at least one checkbox ticked you should see no message and the
'save' button should be enabled. If you make a change (untick a box), the
validation kicks in and things return to the correct state. Sounds like we
need to check the selected values whenever the 'change' icon is clicked

Thanks!

Belén

>
>Note: for the message 'label' the I insert and then show when there are
>no matches, it is guaranteed not to pollute the database because it can
>never be in the checked state.
>
>- David
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-02  9:16 ` Barros Pena, Belen
@ 2015-09-02 14:46   ` Reyna, David
  2015-09-02 15:58     ` Barros Pena, Belen
  0 siblings, 1 reply; 12+ messages in thread
From: Reyna, David @ 2015-09-02 14:46 UTC (permalink / raw)
  To: BARROS PENA, BELEN; +Cc: toaster@yoctoproject.org

Hi Belén,

Thank you for the observation.

I have updated "dreyna/project_fstypes_8126" to address that issue, and the code now always pre-initializes the warning message element so that the previous value is not dangling.

- David


> -----Original Message-----
> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> Sent: Wednesday, September 02, 2015 2:16 AM
> To: Reyna, David
> Cc: toaster@yoctoproject.org
> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are missing
> when "IMAGE_FSTYPES" field is not properly edited
> 
> 
> 
> On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com> wrote:
> 
> >Hi Belén,
> >
> >Please find the patch for 8126 here:
> >
> >   dreyna/project_fstypes_8126
> 
> Hi David,
> 
> This is looking fairly good. I've only run across one problem. This is how
> to reproduce:
> 
> 1. Click the 'change' icon for IMAGE_FSTYPES
> 
> 2. Deselect all values: the 'save' button becomes disabled and the message
> asking you to select at least one image type appears. This is the expected
> behaviour
> 
> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the way it
> was before you clicked the 'change' icon. This is once more the correct
> behaviour
> 
> 4. Now click the 'change' icon again. There are image types selected, but
> the message 'You must select at least one image type' still shows, and the
> 'Save' button is disabled. This is not the correct behaviour. As long as
> there is at least one checkbox ticked you should see no message and the
> 'save' button should be enabled. If you make a change (untick a box), the
> validation kicks in and things return to the correct state. Sounds like we
> need to check the selected values whenever the 'change' icon is clicked
> 
> Thanks!
> 
> Belén
> 
> >
> >Note: for the message 'label' the I insert and then show when there are
> >no matches, it is guaranteed not to pollute the database because it can
> >never be in the checked state.
> >
> >- David
> >
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-02 14:46   ` Reyna, David
@ 2015-09-02 15:58     ` Barros Pena, Belen
  2015-09-14 14:19       ` Michael Wood
  0 siblings, 1 reply; 12+ messages in thread
From: Barros Pena, Belen @ 2015-09-02 15:58 UTC (permalink / raw)
  To: Reyna, David L (Wind River); +Cc: toaster@yoctoproject.org



On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com> wrote:

>Hi Belén,
>
>Thank you for the observation.
>
>I have updated "dreyna/project_fstypes_8126" to address that issue, and
>the code now always pre-initializes the warning message element so that
>the previous value is not dangling.

yep: this seems to behave as expected now. Thanks!

I've also realised that the base branch seems a bit old. Could you rebase
and resubmit?

Cheers

Belén

>
>- David
>
>
>> -----Original Message-----
>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>> Sent: Wednesday, September 02, 2015 2:16 AM
>> To: Reyna, David
>> Cc: toaster@yoctoproject.org
>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are missing
>> when "IMAGE_FSTYPES" field is not properly edited
>>
>>
>>
>> On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com> wrote:
>>
>> >Hi Belén,
>> >
>> >Please find the patch for 8126 here:
>> >
>> >   dreyna/project_fstypes_8126
>>
>> Hi David,
>>
>> This is looking fairly good. I've only run across one problem. This is
>>how
>> to reproduce:
>>
>> 1. Click the 'change' icon for IMAGE_FSTYPES
>>
>> 2. Deselect all values: the 'save' button becomes disabled and the
>>message
>> asking you to select at least one image type appears. This is the
>>expected
>> behaviour
>>
>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the way
>>it
>> was before you clicked the 'change' icon. This is once more the correct
>> behaviour
>>
>> 4. Now click the 'change' icon again. There are image types selected,
>>but
>> the message 'You must select at least one image type' still shows, and
>>the
>> 'Save' button is disabled. This is not the correct behaviour. As long as
>> there is at least one checkbox ticked you should see no message and the
>> 'save' button should be enabled. If you make a change (untick a box),
>>the
>> validation kicks in and things return to the correct state. Sounds like
>>we
>> need to check the selected values whenever the 'change' icon is clicked
>>
>> Thanks!
>>
>> Belén
>>
>> >
>> >Note: for the message 'label' the I insert and then show when there are
>> >no matches, it is guaranteed not to pollute the database because it can
>> >never be in the checked state.
>> >
>> >- David
>> >
>>
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-02 15:58     ` Barros Pena, Belen
@ 2015-09-14 14:19       ` Michael Wood
  2015-09-23  9:02         ` Reyna, David
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Wood @ 2015-09-14 14:19 UTC (permalink / raw)
  To: toaster

On 02/09/15 16:58, Barros Pena, Belen wrote:
>
> On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com> wrote:
>
>> Hi Belén,
>>
>> Thank you for the observation.
>>
>> I have updated "dreyna/project_fstypes_8126" to address that issue, and
>> the code now always pre-initializes the warning message element so that
>> the previous value is not dangling.
> yep: this seems to behave as expected now. Thanks!
>
> I've also realised that the base branch seems a bit old. Could you rebase
> and resubmit?
>
> Cheers
>
> Belén
>
>> - David
>>
>>
>>> -----Original Message-----
>>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>>> Sent: Wednesday, September 02, 2015 2:16 AM
>>> To: Reyna, David
>>> Cc: toaster@yoctoproject.org
>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are missing
>>> when "IMAGE_FSTYPES" field is not properly edited
>>>
>>>
>>>
>>> On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com> wrote:
>>>
>>>> Hi Belén,
>>>>
>>>> Please find the patch for 8126 here:
>>>>
>>>>    dreyna/project_fstypes_8126
>>> Hi David,
>>>
>>> This is looking fairly good. I've only run across one problem. This is
>>> how
>>> to reproduce:
>>>
>>> 1. Click the 'change' icon for IMAGE_FSTYPES
>>>
>>> 2. Deselect all values: the 'save' button becomes disabled and the
>>> message
>>> asking you to select at least one image type appears. This is the
>>> expected
>>> behaviour
>>>
>>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the way
>>> it
>>> was before you clicked the 'change' icon. This is once more the correct
>>> behaviour
>>>
>>> 4. Now click the 'change' icon again. There are image types selected,
>>> but
>>> the message 'You must select at least one image type' still shows, and
>>> the
>>> 'Save' button is disabled. This is not the correct behaviour. As long as
>>> there is at least one checkbox ticked you should see no message and the
>>> 'save' button should be enabled. If you make a change (untick a box),
>>> the
>>> validation kicks in and things return to the correct state. Sounds like
>>> we
>>> need to check the selected values whenever the 'change' icon is clicked
>>>
>>> Thanks!
>>>
>>> Belén
>>>
>>>> Note: for the message 'label' the I insert and then show when there are
>>>> no matches, it is guaranteed not to pollute the database because it can
>>>> never be in the checked state.
>>>>
>>>> - David
>>>>

Some review comments below:

 From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17 00:00:00 2001
From: David Reyna <David.Reyna@windriver.com>
Date: Wed, 2 Sep 2015 15:34:14 -0700
Subject: bitbake: toaster: display warnings for bad "IMAGE_FSTYPES" values

Display warning message for IMAGE_FSTYPES when no value is selected or
when the filter does not have any matches

[YOCTO #8126]

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

diff --git a/bitbake/lib/toaster/toastergui/templates/projectconf.html b/bitbake/lib/toaster/toastergui/templates/projectconf.html
index 4c5a188..b477b4e 100644
--- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
+++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
@@ -43,6 +43,7 @@
                      <input id="filter-image_fstypes" type="text" placeholder="Search image types" class="span4">
                      <div id="all-image_fstypes" class="scrolling">
                      </div>
+                    <span class="help-block" id="fstypes-error-message"></span>
                      <button id="apply-change-image_fstypes" type="button" class="btn">Save</button>
                      <button id="cancel-change-image_fstypes" type="button" class="btn btn-link">Cancel</button>
                  </form>
@@ -312,9 +313,11 @@
              });
              if ( 0 == any_checked ) {

		 $("#apply-change-image_fstypes").attr("disabled","disabled");
+                $('#fstypes-error-message').html("You must select at least one image type");

MW - Given that this error message doesn't change from this text, rather than adding and removing the text add the text to the DOM in the #fstypes-error-message element and .hide .show this makes it easier to update the error message in the html and keeps a slightly cleaner separation between the UI description and the logic of the javascript.

	     }
              else {
                  $("#apply-change-image_fstypes").removeAttr("disabled");
+                $('#fstypes-error-message').html("");
              }
          }
  
@@ -546,10 +549,14 @@
                  // Add the un-checked boxes second
                  for (var i = 0, length = fstypes_list.length; i < length; i++) {
                      if (0  > fstypes.indexOf(" "+fstypes_list[i].value+" ")) {
-                            html += '<label class="checkbox"><input type="checkbox" class="fs-checkbox-fstypes" value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n';
+                        html += '<label class="checkbox"><input type="checkbox" class="fs-checkbox-fstypes" value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n';
                      }
                  }
+                // Add the 'no search matches' line last
+                html += '<label id="no-match-fstypes">No image types found</label>\n';


MW - Rather than appending this to the list just add the text to the #fstypes-error-message and set it it initially to style="display:none" and then toggle it with .hide .show

+                // Display the list
                  document.getElementById("all-image_fstypes").innerHTML = html;
+                $('#no-match-fstypes').hide();
  
                  // Watch elements to disable Save when none are checked
                  $(".fs-checkbox-fstypes").each(function(){
@@ -558,8 +565,9 @@
                      });
                  });
  
-                // clear the previous filter values
+                // clear the previous filter values and warning messages
                  $("input#filter-image_fstypes").val("");
+                $('#fstypes-error-message').html("");
              });
  
              $('#cancel-change-image_fstypes').click(function(){
@@ -569,17 +577,24 @@
              });
  
              $('#filter-image_fstypes').on('input', function(){
-                   var valThis = $(this).val().toLowerCase();
+                var valThis = $(this).val().toLowerCase();
+                var match_count=0;


MW - camelCase please


		     $('#all-image_fstypes label').each(function(){
                      var text = $(this).text().toLowerCase();
                      var match = text.indexOf(valThis);
                      if (match >= 0) {
                          $(this).show();
+                        match_count += 1;
                      }
                      else {
                          $(this).hide();
                      }
                  });
+                if (0 == match_count) {


MW - This should be if (matchCount === 0)
triple equals for full equality and variable on the left



  +                   $('#no-match-fstypes').show();
+                } else {
+                   $('#no-match-fstypes').hide();
+                }
              });
  
              $('#apply-change-image_fstypes').click(function(){
-- 
cgit v0.10.2









^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-14 14:19       ` Michael Wood
@ 2015-09-23  9:02         ` Reyna, David
  2015-09-28 10:11           ` Barros Pena, Belen
  0 siblings, 1 reply; 12+ messages in thread
From: Reyna, David @ 2015-09-23  9:02 UTC (permalink / raw)
  To: WOOD, MICHAEL; +Cc: toaster@yoctoproject.org

Hi Michael,

I have made the requested changes and updated "dreyna/project_fstypes_8126", except for this one:

> +                // Add the 'no search matches' line last
> +                html += '<label id="no-match-fstypes">No image types found</label>\n';
> 
> MW - Rather than appending this to the list just add the text to the #fstypes-error-message 
> and set it it initially to style="display:none" and then toggle it with .hide .show

Belén's design had this message appear within the selection list when there were no search matches instead of in the optional error message below it. That is why I did this implementation. I do toggle it with .hide .show.

- David

> -----Original Message-----
> From: toaster-bounces@yoctoproject.org [mailto:toaster-
> bounces@yoctoproject.org] On Behalf Of Michael Wood
> Sent: Monday, September 14, 2015 7:19 AM
> To: toaster@yoctoproject.org
> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
> missing when "IMAGE_FSTYPES" field is not properly edited
> 
> On 02/09/15 16:58, Barros Pena, Belen wrote:
> >
> > On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com>
> wrote:
> >
> >> Hi Belén,
> >>
> >> Thank you for the observation.
> >>
> >> I have updated "dreyna/project_fstypes_8126" to address that issue,
> and
> >> the code now always pre-initializes the warning message element so
> that
> >> the previous value is not dangling.
> > yep: this seems to behave as expected now. Thanks!
> >
> > I've also realised that the base branch seems a bit old. Could you
> rebase
> > and resubmit?
> >
> > Cheers
> >
> > Belén
> >
> >> - David
> >>
> >>
> >>> -----Original Message-----
> >>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> >>> Sent: Wednesday, September 02, 2015 2:16 AM
> >>> To: Reyna, David
> >>> Cc: toaster@yoctoproject.org
> >>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
> missing
> >>> when "IMAGE_FSTYPES" field is not properly edited
> >>>
> >>>
> >>>
> >>> On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com>
> wrote:
> >>>
> >>>> Hi Belén,
> >>>>
> >>>> Please find the patch for 8126 here:
> >>>>
> >>>>    dreyna/project_fstypes_8126
> >>> Hi David,
> >>>
> >>> This is looking fairly good. I've only run across one problem.
> This is
> >>> how
> >>> to reproduce:
> >>>
> >>> 1. Click the 'change' icon for IMAGE_FSTYPES
> >>>
> >>> 2. Deselect all values: the 'save' button becomes disabled and the
> >>> message
> >>> asking you to select at least one image type appears. This is the
> >>> expected
> >>> behaviour
> >>>
> >>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the
> way
> >>> it
> >>> was before you clicked the 'change' icon. This is once more the
> correct
> >>> behaviour
> >>>
> >>> 4. Now click the 'change' icon again. There are image types
> selected,
> >>> but
> >>> the message 'You must select at least one image type' still shows,
> and
> >>> the
> >>> 'Save' button is disabled. This is not the correct behaviour. As
> long as
> >>> there is at least one checkbox ticked you should see no message
> and the
> >>> 'save' button should be enabled. If you make a change (untick a
> box),
> >>> the
> >>> validation kicks in and things return to the correct state. Sounds
> like
> >>> we
> >>> need to check the selected values whenever the 'change' icon is
> clicked
> >>>
> >>> Thanks!
> >>>
> >>> Belén
> >>>
> >>>> Note: for the message 'label' the I insert and then show when
> there are
> >>>> no matches, it is guaranteed not to pollute the database because
> it can
> >>>> never be in the checked state.
> >>>>
> >>>> - David
> >>>>
> 
> Some review comments below:
> 
>  From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17 00:00:00
> 2001
> From: David Reyna <David.Reyna@windriver.com>
> Date: Wed, 2 Sep 2015 15:34:14 -0700
> Subject: bitbake: toaster: display warnings for bad "IMAGE_FSTYPES"
> values
> 
> Display warning message for IMAGE_FSTYPES when no value is selected or
> when the filter does not have any matches
> 
> [YOCTO #8126]
> 
> Signed-off-by: David Reyna <David.Reyna@windriver.com>
> 
> diff --git a/bitbake/lib/toaster/toastergui/templates/projectconf.html
> b/bitbake/lib/toaster/toastergui/templates/projectconf.html
> index 4c5a188..b477b4e 100644
> --- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
> +++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
> @@ -43,6 +43,7 @@
>                       <input id="filter-image_fstypes" type="text"
> placeholder="Search image types" class="span4">
>                       <div id="all-image_fstypes" class="scrolling">
>                       </div>
> +                    <span class="help-block" id="fstypes-error-
> message"></span>
>                       <button id="apply-change-image_fstypes"
> type="button" class="btn">Save</button>
>                       <button id="cancel-change-image_fstypes"
> type="button" class="btn btn-link">Cancel</button>
>                   </form>
> @@ -312,9 +313,11 @@
>               });
>               if ( 0 == any_checked ) {
> 
> 		 $("#apply-change-
> image_fstypes").attr("disabled","disabled");
> +                $('#fstypes-error-message').html("You must select at
> least one image type");
> 
> MW - Given that this error message doesn't change from this text,
> rather than adding and removing the text add the text to the DOM in
> the #fstypes-error-message element and .hide .show this makes it
> easier to update the error message in the html and keeps a slightly
> cleaner separation between the UI description and the logic of the
> javascript.
> 
> 	     }
>               else {
>                   $("#apply-change-
> image_fstypes").removeAttr("disabled");
> +                $('#fstypes-error-message').html("");
>               }
>           }
> 
> @@ -546,10 +549,14 @@
>                   // Add the un-checked boxes second
>                   for (var i = 0, length = fstypes_list.length; i <
> length; i++) {
>                       if (0  > fstypes.indexOf("
> "+fstypes_list[i].value+" ")) {
> -                            html += '<label class="checkbox"><input
> type="checkbox" class="fs-checkbox-fstypes"
> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
> ;
> +                        html += '<label class="checkbox"><input
> type="checkbox" class="fs-checkbox-fstypes"
> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
> ;
>                       }
>                   }
> +                // Add the 'no search matches' line last
> +                html += '<label id="no-match-fstypes">No image types
> found</label>\n';
> 
> 
> MW - Rather than appending this to the list just add the text to the
> #fstypes-error-message and set it it initially to style="display:none"
> and then toggle it with .hide .show
> 
> +                // Display the list
>                   document.getElementById("all-
> image_fstypes").innerHTML = html;
> +                $('#no-match-fstypes').hide();
> 
>                   // Watch elements to disable Save when none are
> checked
>                   $(".fs-checkbox-fstypes").each(function(){
> @@ -558,8 +565,9 @@
>                       });
>                   });
> 
> -                // clear the previous filter values
> +                // clear the previous filter values and warning
> messages
>                   $("input#filter-image_fstypes").val("");
> +                $('#fstypes-error-message').html("");
>               });
> 
>               $('#cancel-change-image_fstypes').click(function(){
> @@ -569,17 +577,24 @@
>               });
> 
>               $('#filter-image_fstypes').on('input', function(){
> -                   var valThis = $(this).val().toLowerCase();
> +                var valThis = $(this).val().toLowerCase();
> +                var match_count=0;
> 
> 
> MW - camelCase please
> 
> 
> 		     $('#all-image_fstypes label').each(function(){
>                       var text = $(this).text().toLowerCase();
>                       var match = text.indexOf(valThis);
>                       if (match >= 0) {
>                           $(this).show();
> +                        match_count += 1;
>                       }
>                       else {
>                           $(this).hide();
>                       }
>                   });
> +                if (0 == match_count) {
> 
> 
> MW - This should be if (matchCount === 0)
> triple equals for full equality and variable on the left
> 
> 
> 
>   +                   $('#no-match-fstypes').show();
> +                } else {
> +                   $('#no-match-fstypes').hide();
> +                }
>               });
> 
>               $('#apply-change-image_fstypes').click(function(){
> --
> cgit v0.10.2
> 
> 
> 
> 
> 
> 
> 
> --
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-23  9:02         ` Reyna, David
@ 2015-09-28 10:11           ` Barros Pena, Belen
  2015-09-30 10:59             ` Michael Wood
  0 siblings, 1 reply; 12+ messages in thread
From: Barros Pena, Belen @ 2015-09-28 10:11 UTC (permalink / raw)
  To: Reyna, David L (Wind River), Wood, Michael G; +Cc: toaster@yoctoproject.org

Sorry for the delay in looking at this.

On 23/09/2015 10:02, "toaster-bounces@yoctoproject.org on behalf of Reyna,
David" <toaster-bounces@yoctoproject.org on behalf of
david.reyna@windriver.com> wrote:

>Hi Michael,
>
>I have made the requested changes and updated
>"dreyna/project_fstypes_8126", except for this one:
>
>> +                // Add the 'no search matches' line last
>> +                html += '<label id="no-match-fstypes">No image types
>>found</label>\n';
>> 
>> MW - Rather than appending this to the list just add the text to the
>>#fstypes-error-message
>> and set it it initially to style="display:none" and then toggle it with
>>.hide .show
>
>Belén's design had this message appear within the selection list when
>there were no search matches instead of in the optional error message
>below it. That is why I did this implementation. I do toggle it with
>.hide .show.

FWIW, I agree with David on this one. The message has to be as close as
possible to the context of the action. Having the 'no image types found'
next to the buttons would be a bit too far away.

The branch looks good from the UI standpoint.

Cheers

Belén

>
>- David
>
>> -----Original Message-----
>> From: toaster-bounces@yoctoproject.org [mailto:toaster-
>> bounces@yoctoproject.org] On Behalf Of Michael Wood
>> Sent: Monday, September 14, 2015 7:19 AM
>> To: toaster@yoctoproject.org
>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>> missing when "IMAGE_FSTYPES" field is not properly edited
>> 
>> On 02/09/15 16:58, Barros Pena, Belen wrote:
>> >
>> > On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com>
>> wrote:
>> >
>> >> Hi Belén,
>> >>
>> >> Thank you for the observation.
>> >>
>> >> I have updated "dreyna/project_fstypes_8126" to address that issue,
>> and
>> >> the code now always pre-initializes the warning message element so
>> that
>> >> the previous value is not dangling.
>> > yep: this seems to behave as expected now. Thanks!
>> >
>> > I've also realised that the base branch seems a bit old. Could you
>> rebase
>> > and resubmit?
>> >
>> > Cheers
>> >
>> > Belén
>> >
>> >> - David
>> >>
>> >>
>> >>> -----Original Message-----
>> >>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>> >>> Sent: Wednesday, September 02, 2015 2:16 AM
>> >>> To: Reyna, David
>> >>> Cc: toaster@yoctoproject.org
>> >>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>> missing
>> >>> when "IMAGE_FSTYPES" field is not properly edited
>> >>>
>> >>>
>> >>>
>> >>> On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com>
>> wrote:
>> >>>
>> >>>> Hi Belén,
>> >>>>
>> >>>> Please find the patch for 8126 here:
>> >>>>
>> >>>>    dreyna/project_fstypes_8126
>> >>> Hi David,
>> >>>
>> >>> This is looking fairly good. I've only run across one problem.
>> This is
>> >>> how
>> >>> to reproduce:
>> >>>
>> >>> 1. Click the 'change' icon for IMAGE_FSTYPES
>> >>>
>> >>> 2. Deselect all values: the 'save' button becomes disabled and the
>> >>> message
>> >>> asking you to select at least one image type appears. This is the
>> >>> expected
>> >>> behaviour
>> >>>
>> >>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the
>> way
>> >>> it
>> >>> was before you clicked the 'change' icon. This is once more the
>> correct
>> >>> behaviour
>> >>>
>> >>> 4. Now click the 'change' icon again. There are image types
>> selected,
>> >>> but
>> >>> the message 'You must select at least one image type' still shows,
>> and
>> >>> the
>> >>> 'Save' button is disabled. This is not the correct behaviour. As
>> long as
>> >>> there is at least one checkbox ticked you should see no message
>> and the
>> >>> 'save' button should be enabled. If you make a change (untick a
>> box),
>> >>> the
>> >>> validation kicks in and things return to the correct state. Sounds
>> like
>> >>> we
>> >>> need to check the selected values whenever the 'change' icon is
>> clicked
>> >>>
>> >>> Thanks!
>> >>>
>> >>> Belén
>> >>>
>> >>>> Note: for the message 'label' the I insert and then show when
>> there are
>> >>>> no matches, it is guaranteed not to pollute the database because
>> it can
>> >>>> never be in the checked state.
>> >>>>
>> >>>> - David
>> >>>>
>> 
>> Some review comments below:
>> 
>>  From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17 00:00:00
>> 2001
>> From: David Reyna <David.Reyna@windriver.com>
>> Date: Wed, 2 Sep 2015 15:34:14 -0700
>> Subject: bitbake: toaster: display warnings for bad "IMAGE_FSTYPES"
>> values
>> 
>> Display warning message for IMAGE_FSTYPES when no value is selected or
>> when the filter does not have any matches
>> 
>> [YOCTO #8126]
>> 
>> Signed-off-by: David Reyna <David.Reyna@windriver.com>
>> 
>> diff --git a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> index 4c5a188..b477b4e 100644
>> --- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> +++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> @@ -43,6 +43,7 @@
>>                       <input id="filter-image_fstypes" type="text"
>> placeholder="Search image types" class="span4">
>>                       <div id="all-image_fstypes" class="scrolling">
>>                       </div>
>> +                    <span class="help-block" id="fstypes-error-
>> message"></span>
>>                       <button id="apply-change-image_fstypes"
>> type="button" class="btn">Save</button>
>>                       <button id="cancel-change-image_fstypes"
>> type="button" class="btn btn-link">Cancel</button>
>>                   </form>
>> @@ -312,9 +313,11 @@
>>               });
>>               if ( 0 == any_checked ) {
>> 
>> 		 $("#apply-change-
>> image_fstypes").attr("disabled","disabled");
>> +                $('#fstypes-error-message').html("You must select at
>> least one image type");
>> 
>> MW - Given that this error message doesn't change from this text,
>> rather than adding and removing the text add the text to the DOM in
>> the #fstypes-error-message element and .hide .show this makes it
>> easier to update the error message in the html and keeps a slightly
>> cleaner separation between the UI description and the logic of the
>> javascript.
>> 
>> 	     }
>>               else {
>>                   $("#apply-change-
>> image_fstypes").removeAttr("disabled");
>> +                $('#fstypes-error-message').html("");
>>               }
>>           }
>> 
>> @@ -546,10 +549,14 @@
>>                   // Add the un-checked boxes second
>>                   for (var i = 0, length = fstypes_list.length; i <
>> length; i++) {
>>                       if (0  > fstypes.indexOf("
>> "+fstypes_list[i].value+" ")) {
>> -                            html += '<label class="checkbox"><input
>> type="checkbox" class="fs-checkbox-fstypes"
>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>> ;
>> +                        html += '<label class="checkbox"><input
>> type="checkbox" class="fs-checkbox-fstypes"
>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>> ;
>>                       }
>>                   }
>> +                // Add the 'no search matches' line last
>> +                html += '<label id="no-match-fstypes">No image types
>> found</label>\n';
>> 
>> 
>> MW - Rather than appending this to the list just add the text to the
>> #fstypes-error-message and set it it initially to style="display:none"
>> and then toggle it with .hide .show
>> 
>> +                // Display the list
>>                   document.getElementById("all-
>> image_fstypes").innerHTML = html;
>> +                $('#no-match-fstypes').hide();
>> 
>>                   // Watch elements to disable Save when none are
>> checked
>>                   $(".fs-checkbox-fstypes").each(function(){
>> @@ -558,8 +565,9 @@
>>                       });
>>                   });
>> 
>> -                // clear the previous filter values
>> +                // clear the previous filter values and warning
>> messages
>>                   $("input#filter-image_fstypes").val("");
>> +                $('#fstypes-error-message').html("");
>>               });
>> 
>>               $('#cancel-change-image_fstypes').click(function(){
>> @@ -569,17 +577,24 @@
>>               });
>> 
>>               $('#filter-image_fstypes').on('input', function(){
>> -                   var valThis = $(this).val().toLowerCase();
>> +                var valThis = $(this).val().toLowerCase();
>> +                var match_count=0;
>> 
>> 
>> MW - camelCase please
>> 
>> 
>> 		     $('#all-image_fstypes label').each(function(){
>>                       var text = $(this).text().toLowerCase();
>>                       var match = text.indexOf(valThis);
>>                       if (match >= 0) {
>>                           $(this).show();
>> +                        match_count += 1;
>>                       }
>>                       else {
>>                           $(this).hide();
>>                       }
>>                   });
>> +                if (0 == match_count) {
>> 
>> 
>> MW - This should be if (matchCount === 0)
>> triple equals for full equality and variable on the left
>> 
>> 
>> 
>>   +                   $('#no-match-fstypes').show();
>> +                } else {
>> +                   $('#no-match-fstypes').hide();
>> +                }
>>               });
>> 
>>               $('#apply-change-image_fstypes').click(function(){
>> --
>> cgit v0.10.2
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> --
>> _______________________________________________
>> toaster mailing list
>> toaster@yoctoproject.org
>> https://lists.yoctoproject.org/listinfo/toaster
>-- 
>_______________________________________________
>toaster mailing list
>toaster@yoctoproject.org
>https://lists.yoctoproject.org/listinfo/toaster



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-28 10:11           ` Barros Pena, Belen
@ 2015-09-30 10:59             ` Michael Wood
  2015-09-30 11:04               ` Barros Pena, Belen
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Wood @ 2015-09-30 10:59 UTC (permalink / raw)
  To: Barros Pena, Belen, Reyna, David L (Wind River); +Cc: toaster@yoctoproject.org

On 28/09/15 11:11, Barros Pena, Belen wrote:
> Sorry for the delay in looking at this.
>
> On 23/09/2015 10:02, "toaster-bounces@yoctoproject.org on behalf of Reyna,
> David" <toaster-bounces@yoctoproject.org on behalf of
> david.reyna@windriver.com> wrote:
>
>> Hi Michael,
>>
>> I have made the requested changes and updated
>> "dreyna/project_fstypes_8126", except for this one:
>>
>>> +                // Add the 'no search matches' line last
>>> +                html += '<label id="no-match-fstypes">No image types
>>> found</label>\n';
>>>
>>> MW - Rather than appending this to the list just add the text to the
>>> #fstypes-error-message
>>> and set it it initially to style="display:none" and then toggle it with
>>> .hide .show
>> Belén's design had this message appear within the selection list when
>> there were no search matches instead of in the optional error message
>> below it. That is why I did this implementation. I do toggle it with
>> .hide .show.
> FWIW, I agree with David on this one. The message has to be as close as
> possible to the context of the action. Having the 'no image types found'
> next to the buttons would be a bit too far away.
>
> The branch looks good from the UI standpoint.

Oh I was going on this design:
http://www.yoctoproject.org/toaster/localconf.html


>
> Cheers
>
> Belén
>
>> - David
>>
>>> -----Original Message-----
>>> From: toaster-bounces@yoctoproject.org [mailto:toaster-
>>> bounces@yoctoproject.org] On Behalf Of Michael Wood
>>> Sent: Monday, September 14, 2015 7:19 AM
>>> To: toaster@yoctoproject.org
>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>>> missing when "IMAGE_FSTYPES" field is not properly edited
>>>
>>> On 02/09/15 16:58, Barros Pena, Belen wrote:
>>>> On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com>
>>> wrote:
>>>>> Hi Belén,
>>>>>
>>>>> Thank you for the observation.
>>>>>
>>>>> I have updated "dreyna/project_fstypes_8126" to address that issue,
>>> and
>>>>> the code now always pre-initializes the warning message element so
>>> that
>>>>> the previous value is not dangling.
>>>> yep: this seems to behave as expected now. Thanks!
>>>>
>>>> I've also realised that the base branch seems a bit old. Could you
>>> rebase
>>>> and resubmit?
>>>>
>>>> Cheers
>>>>
>>>> Belén
>>>>
>>>>> - David
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>>>>>> Sent: Wednesday, September 02, 2015 2:16 AM
>>>>>> To: Reyna, David
>>>>>> Cc: toaster@yoctoproject.org
>>>>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>>> missing
>>>>>> when "IMAGE_FSTYPES" field is not properly edited
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com>
>>> wrote:
>>>>>>> Hi Belén,
>>>>>>>
>>>>>>> Please find the patch for 8126 here:
>>>>>>>
>>>>>>>     dreyna/project_fstypes_8126
>>>>>> Hi David,
>>>>>>
>>>>>> This is looking fairly good. I've only run across one problem.
>>> This is
>>>>>> how
>>>>>> to reproduce:
>>>>>>
>>>>>> 1. Click the 'change' icon for IMAGE_FSTYPES
>>>>>>
>>>>>> 2. Deselect all values: the 'save' button becomes disabled and the
>>>>>> message
>>>>>> asking you to select at least one image type appears. This is the
>>>>>> expected
>>>>>> behaviour
>>>>>>
>>>>>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the
>>> way
>>>>>> it
>>>>>> was before you clicked the 'change' icon. This is once more the
>>> correct
>>>>>> behaviour
>>>>>>
>>>>>> 4. Now click the 'change' icon again. There are image types
>>> selected,
>>>>>> but
>>>>>> the message 'You must select at least one image type' still shows,
>>> and
>>>>>> the
>>>>>> 'Save' button is disabled. This is not the correct behaviour. As
>>> long as
>>>>>> there is at least one checkbox ticked you should see no message
>>> and the
>>>>>> 'save' button should be enabled. If you make a change (untick a
>>> box),
>>>>>> the
>>>>>> validation kicks in and things return to the correct state. Sounds
>>> like
>>>>>> we
>>>>>> need to check the selected values whenever the 'change' icon is
>>> clicked
>>>>>> Thanks!
>>>>>>
>>>>>> Belén
>>>>>>
>>>>>>> Note: for the message 'label' the I insert and then show when
>>> there are
>>>>>>> no matches, it is guaranteed not to pollute the database because
>>> it can
>>>>>>> never be in the checked state.
>>>>>>>
>>>>>>> - David
>>>>>>>
>>> Some review comments below:
>>>
>>>   From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17 00:00:00
>>> 2001
>>> From: David Reyna <David.Reyna@windriver.com>
>>> Date: Wed, 2 Sep 2015 15:34:14 -0700
>>> Subject: bitbake: toaster: display warnings for bad "IMAGE_FSTYPES"
>>> values
>>>
>>> Display warning message for IMAGE_FSTYPES when no value is selected or
>>> when the filter does not have any matches
>>>
>>> [YOCTO #8126]
>>>
>>> Signed-off-by: David Reyna <David.Reyna@windriver.com>
>>>
>>> diff --git a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>> b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>> index 4c5a188..b477b4e 100644
>>> --- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>> +++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>> @@ -43,6 +43,7 @@
>>>                        <input id="filter-image_fstypes" type="text"
>>> placeholder="Search image types" class="span4">
>>>                        <div id="all-image_fstypes" class="scrolling">
>>>                        </div>
>>> +                    <span class="help-block" id="fstypes-error-
>>> message"></span>
>>>                        <button id="apply-change-image_fstypes"
>>> type="button" class="btn">Save</button>
>>>                        <button id="cancel-change-image_fstypes"
>>> type="button" class="btn btn-link">Cancel</button>
>>>                    </form>
>>> @@ -312,9 +313,11 @@
>>>                });
>>>                if ( 0 == any_checked ) {
>>>
>>> 		 $("#apply-change-
>>> image_fstypes").attr("disabled","disabled");
>>> +                $('#fstypes-error-message').html("You must select at
>>> least one image type");
>>>
>>> MW - Given that this error message doesn't change from this text,
>>> rather than adding and removing the text add the text to the DOM in
>>> the #fstypes-error-message element and .hide .show this makes it
>>> easier to update the error message in the html and keeps a slightly
>>> cleaner separation between the UI description and the logic of the
>>> javascript.
>>>
>>> 	     }
>>>                else {
>>>                    $("#apply-change-
>>> image_fstypes").removeAttr("disabled");
>>> +                $('#fstypes-error-message').html("");
>>>                }
>>>            }
>>>
>>> @@ -546,10 +549,14 @@
>>>                    // Add the un-checked boxes second
>>>                    for (var i = 0, length = fstypes_list.length; i <
>>> length; i++) {
>>>                        if (0  > fstypes.indexOf("
>>> "+fstypes_list[i].value+" ")) {
>>> -                            html += '<label class="checkbox"><input
>>> type="checkbox" class="fs-checkbox-fstypes"
>>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>>> ;
>>> +                        html += '<label class="checkbox"><input
>>> type="checkbox" class="fs-checkbox-fstypes"
>>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>>> ;
>>>                        }
>>>                    }
>>> +                // Add the 'no search matches' line last
>>> +                html += '<label id="no-match-fstypes">No image types
>>> found</label>\n';
>>>
>>>
>>> MW - Rather than appending this to the list just add the text to the
>>> #fstypes-error-message and set it it initially to style="display:none"
>>> and then toggle it with .hide .show
>>>
>>> +                // Display the list
>>>                    document.getElementById("all-
>>> image_fstypes").innerHTML = html;
>>> +                $('#no-match-fstypes').hide();
>>>
>>>                    // Watch elements to disable Save when none are
>>> checked
>>>                    $(".fs-checkbox-fstypes").each(function(){
>>> @@ -558,8 +565,9 @@
>>>                        });
>>>                    });
>>>
>>> -                // clear the previous filter values
>>> +                // clear the previous filter values and warning
>>> messages
>>>                    $("input#filter-image_fstypes").val("");
>>> +                $('#fstypes-error-message').html("");
>>>                });
>>>
>>>                $('#cancel-change-image_fstypes').click(function(){
>>> @@ -569,17 +577,24 @@
>>>                });
>>>
>>>                $('#filter-image_fstypes').on('input', function(){
>>> -                   var valThis = $(this).val().toLowerCase();
>>> +                var valThis = $(this).val().toLowerCase();
>>> +                var match_count=0;
>>>
>>>
>>> MW - camelCase please
>>>
>>>
>>> 		     $('#all-image_fstypes label').each(function(){
>>>                        var text = $(this).text().toLowerCase();
>>>                        var match = text.indexOf(valThis);
>>>                        if (match >= 0) {
>>>                            $(this).show();
>>> +                        match_count += 1;
>>>                        }
>>>                        else {
>>>                            $(this).hide();
>>>                        }
>>>                    });
>>> +                if (0 == match_count) {
>>>
>>>
>>> MW - This should be if (matchCount === 0)
>>> triple equals for full equality and variable on the left
>>>
>>>
>>>
>>>    +                   $('#no-match-fstypes').show();
>>> +                } else {
>>> +                   $('#no-match-fstypes').hide();
>>> +                }
>>>                });
>>>
>>>                $('#apply-change-image_fstypes').click(function(){
>>> --
>>> cgit v0.10.2
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> --
>>> _______________________________________________
>>> toaster mailing list
>>> toaster@yoctoproject.org
>>> https://lists.yoctoproject.org/listinfo/toaster
>> -- 
>> _______________________________________________
>> toaster mailing list
>> toaster@yoctoproject.org
>> https://lists.yoctoproject.org/listinfo/toaster



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-30 10:59             ` Michael Wood
@ 2015-09-30 11:04               ` Barros Pena, Belen
  2015-09-30 13:47                 ` Reyna, David
  0 siblings, 1 reply; 12+ messages in thread
From: Barros Pena, Belen @ 2015-09-30 11:04 UTC (permalink / raw)
  To: Wood, Michael G, Reyna, David L (Wind River); +Cc: toaster@yoctoproject.org



On 30/09/2015 11:59, "Michael Wood" <michael.g.wood@intel.com> wrote:

>On 28/09/15 11:11, Barros Pena, Belen wrote:
>> Sorry for the delay in looking at this.
>>
>> On 23/09/2015 10:02, "toaster-bounces@yoctoproject.org on behalf of
>>Reyna,
>> David" <toaster-bounces@yoctoproject.org on behalf of
>> david.reyna@windriver.com> wrote:
>>
>>> Hi Michael,
>>>
>>> I have made the requested changes and updated
>>> "dreyna/project_fstypes_8126", except for this one:
>>>
>>>> +                // Add the 'no search matches' line last
>>>> +                html += '<label id="no-match-fstypes">No image types
>>>> found</label>\n';
>>>>
>>>> MW - Rather than appending this to the list just add the text to the
>>>> #fstypes-error-message
>>>> and set it it initially to style="display:none" and then toggle it
>>>>with
>>>> .hide .show
>>> Belén's design had this message appear within the selection list when
>>> there were no search matches instead of in the optional error message
>>> below it. That is why I did this implementation. I do toggle it with
>>> .hide .show.
>> FWIW, I agree with David on this one. The message has to be as close as
>> possible to the context of the action. Having the 'no image types found'
>> next to the buttons would be a bit too far away.
>>
>> The branch looks good from the UI standpoint.
>
>Oh I was going on this design:
>http://www.yoctoproject.org/toaster/localconf.html

Ah, I see what you mean know. The prototype reflects the changes that will
be brought in by 

https://bugzilla.yoctoproject.org/show_bug.cgi?id=7828

But David is working on the state before that one. Sorry for the confusion.

Belén

>
>
>>
>> Cheers
>>
>> Belén
>>
>>> - David
>>>
>>>> -----Original Message-----
>>>> From: toaster-bounces@yoctoproject.org [mailto:toaster-
>>>> bounces@yoctoproject.org] On Behalf Of Michael Wood
>>>> Sent: Monday, September 14, 2015 7:19 AM
>>>> To: toaster@yoctoproject.org
>>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>>>> missing when "IMAGE_FSTYPES" field is not properly edited
>>>>
>>>> On 02/09/15 16:58, Barros Pena, Belen wrote:
>>>>> On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com>
>>>> wrote:
>>>>>> Hi Belén,
>>>>>>
>>>>>> Thank you for the observation.
>>>>>>
>>>>>> I have updated "dreyna/project_fstypes_8126" to address that issue,
>>>> and
>>>>>> the code now always pre-initializes the warning message element so
>>>> that
>>>>>> the previous value is not dangling.
>>>>> yep: this seems to behave as expected now. Thanks!
>>>>>
>>>>> I've also realised that the base branch seems a bit old. Could you
>>>> rebase
>>>>> and resubmit?
>>>>>
>>>>> Cheers
>>>>>
>>>>> Belén
>>>>>
>>>>>> - David
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>>>>>>> Sent: Wednesday, September 02, 2015 2:16 AM
>>>>>>> To: Reyna, David
>>>>>>> Cc: toaster@yoctoproject.org
>>>>>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>>>> missing
>>>>>>> when "IMAGE_FSTYPES" field is not properly edited
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 02/09/2015 08:44, "Reyna, David" <david.reyna@windriver.com>
>>>> wrote:
>>>>>>>> Hi Belén,
>>>>>>>>
>>>>>>>> Please find the patch for 8126 here:
>>>>>>>>
>>>>>>>>     dreyna/project_fstypes_8126
>>>>>>> Hi David,
>>>>>>>
>>>>>>> This is looking fairly good. I've only run across one problem.
>>>> This is
>>>>>>> how
>>>>>>> to reproduce:
>>>>>>>
>>>>>>> 1. Click the 'change' icon for IMAGE_FSTYPES
>>>>>>>
>>>>>>> 2. Deselect all values: the 'save' button becomes disabled and the
>>>>>>> message
>>>>>>> asking you to select at least one image type appears. This is the
>>>>>>> expected
>>>>>>> behaviour
>>>>>>>
>>>>>>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays the
>>>> way
>>>>>>> it
>>>>>>> was before you clicked the 'change' icon. This is once more the
>>>> correct
>>>>>>> behaviour
>>>>>>>
>>>>>>> 4. Now click the 'change' icon again. There are image types
>>>> selected,
>>>>>>> but
>>>>>>> the message 'You must select at least one image type' still shows,
>>>> and
>>>>>>> the
>>>>>>> 'Save' button is disabled. This is not the correct behaviour. As
>>>> long as
>>>>>>> there is at least one checkbox ticked you should see no message
>>>> and the
>>>>>>> 'save' button should be enabled. If you make a change (untick a
>>>> box),
>>>>>>> the
>>>>>>> validation kicks in and things return to the correct state. Sounds
>>>> like
>>>>>>> we
>>>>>>> need to check the selected values whenever the 'change' icon is
>>>> clicked
>>>>>>> Thanks!
>>>>>>>
>>>>>>> Belén
>>>>>>>
>>>>>>>> Note: for the message 'label' the I insert and then show when
>>>> there are
>>>>>>>> no matches, it is guaranteed not to pollute the database because
>>>> it can
>>>>>>>> never be in the checked state.
>>>>>>>>
>>>>>>>> - David
>>>>>>>>
>>>> Some review comments below:
>>>>
>>>>   From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17 00:00:00
>>>> 2001
>>>> From: David Reyna <David.Reyna@windriver.com>
>>>> Date: Wed, 2 Sep 2015 15:34:14 -0700
>>>> Subject: bitbake: toaster: display warnings for bad "IMAGE_FSTYPES"
>>>> values
>>>>
>>>> Display warning message for IMAGE_FSTYPES when no value is selected or
>>>> when the filter does not have any matches
>>>>
>>>> [YOCTO #8126]
>>>>
>>>> Signed-off-by: David Reyna <David.Reyna@windriver.com>
>>>>
>>>> diff --git a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>> b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>> index 4c5a188..b477b4e 100644
>>>> --- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>> +++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>> @@ -43,6 +43,7 @@
>>>>                        <input id="filter-image_fstypes" type="text"
>>>> placeholder="Search image types" class="span4">
>>>>                        <div id="all-image_fstypes" class="scrolling">
>>>>                        </div>
>>>> +                    <span class="help-block" id="fstypes-error-
>>>> message"></span>
>>>>                        <button id="apply-change-image_fstypes"
>>>> type="button" class="btn">Save</button>
>>>>                        <button id="cancel-change-image_fstypes"
>>>> type="button" class="btn btn-link">Cancel</button>
>>>>                    </form>
>>>> @@ -312,9 +313,11 @@
>>>>                });
>>>>                if ( 0 == any_checked ) {
>>>>
>>>> 		 $("#apply-change-
>>>> image_fstypes").attr("disabled","disabled");
>>>> +                $('#fstypes-error-message').html("You must select at
>>>> least one image type");
>>>>
>>>> MW - Given that this error message doesn't change from this text,
>>>> rather than adding and removing the text add the text to the DOM in
>>>> the #fstypes-error-message element and .hide .show this makes it
>>>> easier to update the error message in the html and keeps a slightly
>>>> cleaner separation between the UI description and the logic of the
>>>> javascript.
>>>>
>>>> 	     }
>>>>                else {
>>>>                    $("#apply-change-
>>>> image_fstypes").removeAttr("disabled");
>>>> +                $('#fstypes-error-message').html("");
>>>>                }
>>>>            }
>>>>
>>>> @@ -546,10 +549,14 @@
>>>>                    // Add the un-checked boxes second
>>>>                    for (var i = 0, length = fstypes_list.length; i <
>>>> length; i++) {
>>>>                        if (0  > fstypes.indexOf("
>>>> "+fstypes_list[i].value+" ")) {
>>>> -                            html += '<label class="checkbox"><input
>>>> type="checkbox" class="fs-checkbox-fstypes"
>>>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>>>> ;
>>>> +                        html += '<label class="checkbox"><input
>>>> type="checkbox" class="fs-checkbox-fstypes"
>>>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>>>> ;
>>>>                        }
>>>>                    }
>>>> +                // Add the 'no search matches' line last
>>>> +                html += '<label id="no-match-fstypes">No image types
>>>> found</label>\n';
>>>>
>>>>
>>>> MW - Rather than appending this to the list just add the text to the
>>>> #fstypes-error-message and set it it initially to style="display:none"
>>>> and then toggle it with .hide .show
>>>>
>>>> +                // Display the list
>>>>                    document.getElementById("all-
>>>> image_fstypes").innerHTML = html;
>>>> +                $('#no-match-fstypes').hide();
>>>>
>>>>                    // Watch elements to disable Save when none are
>>>> checked
>>>>                    $(".fs-checkbox-fstypes").each(function(){
>>>> @@ -558,8 +565,9 @@
>>>>                        });
>>>>                    });
>>>>
>>>> -                // clear the previous filter values
>>>> +                // clear the previous filter values and warning
>>>> messages
>>>>                    $("input#filter-image_fstypes").val("");
>>>> +                $('#fstypes-error-message').html("");
>>>>                });
>>>>
>>>>                $('#cancel-change-image_fstypes').click(function(){
>>>> @@ -569,17 +577,24 @@
>>>>                });
>>>>
>>>>                $('#filter-image_fstypes').on('input', function(){
>>>> -                   var valThis = $(this).val().toLowerCase();
>>>> +                var valThis = $(this).val().toLowerCase();
>>>> +                var match_count=0;
>>>>
>>>>
>>>> MW - camelCase please
>>>>
>>>>
>>>> 		     $('#all-image_fstypes label').each(function(){
>>>>                        var text = $(this).text().toLowerCase();
>>>>                        var match = text.indexOf(valThis);
>>>>                        if (match >= 0) {
>>>>                            $(this).show();
>>>> +                        match_count += 1;
>>>>                        }
>>>>                        else {
>>>>                            $(this).hide();
>>>>                        }
>>>>                    });
>>>> +                if (0 == match_count) {
>>>>
>>>>
>>>> MW - This should be if (matchCount === 0)
>>>> triple equals for full equality and variable on the left
>>>>
>>>>
>>>>
>>>>    +                   $('#no-match-fstypes').show();
>>>> +                } else {
>>>> +                   $('#no-match-fstypes').hide();
>>>> +                }
>>>>                });
>>>>
>>>>                $('#apply-change-image_fstypes').click(function(){
>>>> --
>>>> cgit v0.10.2
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> _______________________________________________
>>>> toaster mailing list
>>>> toaster@yoctoproject.org
>>>> https://lists.yoctoproject.org/listinfo/toaster
>>> -- 
>>> _______________________________________________
>>> toaster mailing list
>>> toaster@yoctoproject.org
>>> https://lists.yoctoproject.org/listinfo/toaster
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-30 11:04               ` Barros Pena, Belen
@ 2015-09-30 13:47                 ` Reyna, David
  2015-09-30 14:05                   ` Barros Pena, Belen
  0 siblings, 1 reply; 12+ messages in thread
From: Reyna, David @ 2015-09-30 13:47 UTC (permalink / raw)
  To: BARROS PENA, BELEN, WOOD, MICHAEL; +Cc: toaster@yoctoproject.org

Hi Belén and Michael,

> But David is working on the state before that one. Sorry for the
> confusion.

Is there an action for me? 

What I have now in the commit does work. I like the original design for handling the "no options selected" in that (a) the message is clear and in-context, and (b) the display does not jump.

- David

> -----Original Message-----
> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> Sent: Wednesday, September 30, 2015 4:04 AM
> To: WOOD, MICHAEL; Reyna, David
> Cc: toaster@yoctoproject.org
> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
> missing when "IMAGE_FSTYPES" field is not properly edited
> 
> 
> 
> On 30/09/2015 11:59, "Michael Wood" <michael.g.wood@intel.com> wrote:
> 
> >On 28/09/15 11:11, Barros Pena, Belen wrote:
> >> Sorry for the delay in looking at this.
> >>
> >> On 23/09/2015 10:02, "toaster-bounces@yoctoproject.org on behalf of
> >>Reyna,
> >> David" <toaster-bounces@yoctoproject.org on behalf of
> >> david.reyna@windriver.com> wrote:
> >>
> >>> Hi Michael,
> >>>
> >>> I have made the requested changes and updated
> >>> "dreyna/project_fstypes_8126", except for this one:
> >>>
> >>>> +                // Add the 'no search matches' line last
> >>>> +                html += '<label id="no-match-fstypes">No image
> types
> >>>> found</label>\n';
> >>>>
> >>>> MW - Rather than appending this to the list just add the text to
> the
> >>>> #fstypes-error-message
> >>>> and set it it initially to style="display:none" and then toggle
> it
> >>>>with
> >>>> .hide .show
> >>> Belén's design had this message appear within the selection list
> when
> >>> there were no search matches instead of in the optional error
> message
> >>> below it. That is why I did this implementation. I do toggle it
> with
> >>> .hide .show.
> >> FWIW, I agree with David on this one. The message has to be as
> close as
> >> possible to the context of the action. Having the 'no image types
> found'
> >> next to the buttons would be a bit too far away.
> >>
> >> The branch looks good from the UI standpoint.
> >
> >Oh I was going on this design:
> >http://www.yoctoproject.org/toaster/localconf.html
> 
> Ah, I see what you mean know. The prototype reflects the changes that
> will
> be brought in by
> 
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=7828
> 
> But David is working on the state before that one. Sorry for the
> confusion.
> 
> Belén
> 
> >
> >
> >>
> >> Cheers
> >>
> >> Belén
> >>
> >>> - David
> >>>
> >>>> -----Original Message-----
> >>>> From: toaster-bounces@yoctoproject.org [mailto:toaster-
> >>>> bounces@yoctoproject.org] On Behalf Of Michael Wood
> >>>> Sent: Monday, September 14, 2015 7:19 AM
> >>>> To: toaster@yoctoproject.org
> >>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
> >>>> missing when "IMAGE_FSTYPES" field is not properly edited
> >>>>
> >>>> On 02/09/15 16:58, Barros Pena, Belen wrote:
> >>>>> On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com>
> >>>> wrote:
> >>>>>> Hi Belén,
> >>>>>>
> >>>>>> Thank you for the observation.
> >>>>>>
> >>>>>> I have updated "dreyna/project_fstypes_8126" to address that
> issue,
> >>>> and
> >>>>>> the code now always pre-initializes the warning message element
> so
> >>>> that
> >>>>>> the previous value is not dangling.
> >>>>> yep: this seems to behave as expected now. Thanks!
> >>>>>
> >>>>> I've also realised that the base branch seems a bit old. Could
> you
> >>>> rebase
> >>>>> and resubmit?
> >>>>>
> >>>>> Cheers
> >>>>>
> >>>>> Belén
> >>>>>
> >>>>>> - David
> >>>>>>
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
> >>>>>>> Sent: Wednesday, September 02, 2015 2:16 AM
> >>>>>>> To: Reyna, David
> >>>>>>> Cc: toaster@yoctoproject.org
> >>>>>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages
> are
> >>>> missing
> >>>>>>> when "IMAGE_FSTYPES" field is not properly edited
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On 02/09/2015 08:44, "Reyna, David"
> <david.reyna@windriver.com>
> >>>> wrote:
> >>>>>>>> Hi Belén,
> >>>>>>>>
> >>>>>>>> Please find the patch for 8126 here:
> >>>>>>>>
> >>>>>>>>     dreyna/project_fstypes_8126
> >>>>>>> Hi David,
> >>>>>>>
> >>>>>>> This is looking fairly good. I've only run across one problem.
> >>>> This is
> >>>>>>> how
> >>>>>>> to reproduce:
> >>>>>>>
> >>>>>>> 1. Click the 'change' icon for IMAGE_FSTYPES
> >>>>>>>
> >>>>>>> 2. Deselect all values: the 'save' button becomes disabled and
> the
> >>>>>>> message
> >>>>>>> asking you to select at least one image type appears. This is
> the
> >>>>>>> expected
> >>>>>>> behaviour
> >>>>>>>
> >>>>>>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays
> the
> >>>> way
> >>>>>>> it
> >>>>>>> was before you clicked the 'change' icon. This is once more
> the
> >>>> correct
> >>>>>>> behaviour
> >>>>>>>
> >>>>>>> 4. Now click the 'change' icon again. There are image types
> >>>> selected,
> >>>>>>> but
> >>>>>>> the message 'You must select at least one image type' still
> shows,
> >>>> and
> >>>>>>> the
> >>>>>>> 'Save' button is disabled. This is not the correct behaviour.
> As
> >>>> long as
> >>>>>>> there is at least one checkbox ticked you should see no
> message
> >>>> and the
> >>>>>>> 'save' button should be enabled. If you make a change (untick
> a
> >>>> box),
> >>>>>>> the
> >>>>>>> validation kicks in and things return to the correct state.
> Sounds
> >>>> like
> >>>>>>> we
> >>>>>>> need to check the selected values whenever the 'change' icon
> is
> >>>> clicked
> >>>>>>> Thanks!
> >>>>>>>
> >>>>>>> Belén
> >>>>>>>
> >>>>>>>> Note: for the message 'label' the I insert and then show when
> >>>> there are
> >>>>>>>> no matches, it is guaranteed not to pollute the database
> because
> >>>> it can
> >>>>>>>> never be in the checked state.
> >>>>>>>>
> >>>>>>>> - David
> >>>>>>>>
> >>>> Some review comments below:
> >>>>
> >>>>   From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17
> 00:00:00
> >>>> 2001
> >>>> From: David Reyna <David.Reyna@windriver.com>
> >>>> Date: Wed, 2 Sep 2015 15:34:14 -0700
> >>>> Subject: bitbake: toaster: display warnings for bad
> "IMAGE_FSTYPES"
> >>>> values
> >>>>
> >>>> Display warning message for IMAGE_FSTYPES when no value is
> selected or
> >>>> when the filter does not have any matches
> >>>>
> >>>> [YOCTO #8126]
> >>>>
> >>>> Signed-off-by: David Reyna <David.Reyna@windriver.com>
> >>>>
> >>>> diff --git
> a/bitbake/lib/toaster/toastergui/templates/projectconf.html
> >>>> b/bitbake/lib/toaster/toastergui/templates/projectconf.html
> >>>> index 4c5a188..b477b4e 100644
> >>>> --- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
> >>>> +++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
> >>>> @@ -43,6 +43,7 @@
> >>>>                        <input id="filter-image_fstypes"
> type="text"
> >>>> placeholder="Search image types" class="span4">
> >>>>                        <div id="all-image_fstypes"
> class="scrolling">
> >>>>                        </div>
> >>>> +                    <span class="help-block" id="fstypes-error-
> >>>> message"></span>
> >>>>                        <button id="apply-change-image_fstypes"
> >>>> type="button" class="btn">Save</button>
> >>>>                        <button id="cancel-change-image_fstypes"
> >>>> type="button" class="btn btn-link">Cancel</button>
> >>>>                    </form>
> >>>> @@ -312,9 +313,11 @@
> >>>>                });
> >>>>                if ( 0 == any_checked ) {
> >>>>
> >>>> 		 $("#apply-change-
> >>>> image_fstypes").attr("disabled","disabled");
> >>>> +                $('#fstypes-error-message').html("You must
> select at
> >>>> least one image type");
> >>>>
> >>>> MW - Given that this error message doesn't change from this text,
> >>>> rather than adding and removing the text add the text to the DOM
> in
> >>>> the #fstypes-error-message element and .hide .show this makes it
> >>>> easier to update the error message in the html and keeps a
> slightly
> >>>> cleaner separation between the UI description and the logic of
> the
> >>>> javascript.
> >>>>
> >>>> 	     }
> >>>>                else {
> >>>>                    $("#apply-change-
> >>>> image_fstypes").removeAttr("disabled");
> >>>> +                $('#fstypes-error-message').html("");
> >>>>                }
> >>>>            }
> >>>>
> >>>> @@ -546,10 +549,14 @@
> >>>>                    // Add the un-checked boxes second
> >>>>                    for (var i = 0, length = fstypes_list.length;
> i <
> >>>> length; i++) {
> >>>>                        if (0  > fstypes.indexOf("
> >>>> "+fstypes_list[i].value+" ")) {
> >>>> -                            html += '<label
> class="checkbox"><input
> >>>> type="checkbox" class="fs-checkbox-fstypes"
> >>>>
> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
> >>>> ;
> >>>> +                        html += '<label class="checkbox"><input
> >>>> type="checkbox" class="fs-checkbox-fstypes"
> >>>>
> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
> >>>> ;
> >>>>                        }
> >>>>                    }
> >>>> +                // Add the 'no search matches' line last
> >>>> +                html += '<label id="no-match-fstypes">No image
> types
> >>>> found</label>\n';
> >>>>
> >>>>
> >>>> MW - Rather than appending this to the list just add the text to
> the
> >>>> #fstypes-error-message and set it it initially to
> style="display:none"
> >>>> and then toggle it with .hide .show
> >>>>
> >>>> +                // Display the list
> >>>>                    document.getElementById("all-
> >>>> image_fstypes").innerHTML = html;
> >>>> +                $('#no-match-fstypes').hide();
> >>>>
> >>>>                    // Watch elements to disable Save when none
> are
> >>>> checked
> >>>>                    $(".fs-checkbox-fstypes").each(function(){
> >>>> @@ -558,8 +565,9 @@
> >>>>                        });
> >>>>                    });
> >>>>
> >>>> -                // clear the previous filter values
> >>>> +                // clear the previous filter values and warning
> >>>> messages
> >>>>                    $("input#filter-image_fstypes").val("");
> >>>> +                $('#fstypes-error-message').html("");
> >>>>                });
> >>>>
> >>>>                $('#cancel-change-
> image_fstypes').click(function(){
> >>>> @@ -569,17 +577,24 @@
> >>>>                });
> >>>>
> >>>>                $('#filter-image_fstypes').on('input', function(){
> >>>> -                   var valThis = $(this).val().toLowerCase();
> >>>> +                var valThis = $(this).val().toLowerCase();
> >>>> +                var match_count=0;
> >>>>
> >>>>
> >>>> MW - camelCase please
> >>>>
> >>>>
> >>>> 		     $('#all-image_fstypes label').each(function(){
> >>>>                        var text = $(this).text().toLowerCase();
> >>>>                        var match = text.indexOf(valThis);
> >>>>                        if (match >= 0) {
> >>>>                            $(this).show();
> >>>> +                        match_count += 1;
> >>>>                        }
> >>>>                        else {
> >>>>                            $(this).hide();
> >>>>                        }
> >>>>                    });
> >>>> +                if (0 == match_count) {
> >>>>
> >>>>
> >>>> MW - This should be if (matchCount === 0)
> >>>> triple equals for full equality and variable on the left
> >>>>
> >>>>
> >>>>
> >>>>    +                   $('#no-match-fstypes').show();
> >>>> +                } else {
> >>>> +                   $('#no-match-fstypes').hide();
> >>>> +                }
> >>>>                });
> >>>>
> >>>>                $('#apply-change-image_fstypes').click(function(){
> >>>> --
> >>>> cgit v0.10.2
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> _______________________________________________
> >>>> toaster mailing list
> >>>> toaster@yoctoproject.org
> >>>> https://lists.yoctoproject.org/listinfo/toaster
> >>> --
> >>> _______________________________________________
> >>> toaster mailing list
> >>> toaster@yoctoproject.org
> >>> https://lists.yoctoproject.org/listinfo/toaster
> >
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-30 13:47                 ` Reyna, David
@ 2015-09-30 14:05                   ` Barros Pena, Belen
  2015-10-06 15:44                     ` Michael Wood
  0 siblings, 1 reply; 12+ messages in thread
From: Barros Pena, Belen @ 2015-09-30 14:05 UTC (permalink / raw)
  To: Reyna, David L (Wind River), Wood, Michael G; +Cc: toaster@yoctoproject.org



On 30/09/2015 14:47, "Reyna, David" <david.reyna@windriver.com> wrote:

>Hi Belén and Michael,
>
>> But David is working on the state before that one. Sorry for the
>> confusion.
>
>Is there an action for me?

Not from the UI side of things. I am not sure about the code.

Thanks!

Belén

>
>What I have now in the commit does work. I like the original design for
>handling the "no options selected" in that (a) the message is clear and
>in-context, and (b) the display does not jump.
>
>- David
>
>> -----Original Message-----
>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>> Sent: Wednesday, September 30, 2015 4:04 AM
>> To: WOOD, MICHAEL; Reyna, David
>> Cc: toaster@yoctoproject.org
>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>> missing when "IMAGE_FSTYPES" field is not properly edited
>>
>>
>>
>> On 30/09/2015 11:59, "Michael Wood" <michael.g.wood@intel.com> wrote:
>>
>> >On 28/09/15 11:11, Barros Pena, Belen wrote:
>> >> Sorry for the delay in looking at this.
>> >>
>> >> On 23/09/2015 10:02, "toaster-bounces@yoctoproject.org on behalf of
>> >>Reyna,
>> >> David" <toaster-bounces@yoctoproject.org on behalf of
>> >> david.reyna@windriver.com> wrote:
>> >>
>> >>> Hi Michael,
>> >>>
>> >>> I have made the requested changes and updated
>> >>> "dreyna/project_fstypes_8126", except for this one:
>> >>>
>> >>>> +                // Add the 'no search matches' line last
>> >>>> +                html += '<label id="no-match-fstypes">No image
>> types
>> >>>> found</label>\n';
>> >>>>
>> >>>> MW - Rather than appending this to the list just add the text to
>> the
>> >>>> #fstypes-error-message
>> >>>> and set it it initially to style="display:none" and then toggle
>> it
>> >>>>with
>> >>>> .hide .show
>> >>> Belén's design had this message appear within the selection list
>> when
>> >>> there were no search matches instead of in the optional error
>> message
>> >>> below it. That is why I did this implementation. I do toggle it
>> with
>> >>> .hide .show.
>> >> FWIW, I agree with David on this one. The message has to be as
>> close as
>> >> possible to the context of the action. Having the 'no image types
>> found'
>> >> next to the buttons would be a bit too far away.
>> >>
>> >> The branch looks good from the UI standpoint.
>> >
>> >Oh I was going on this design:
>> >http://www.yoctoproject.org/toaster/localconf.html
>>
>> Ah, I see what you mean know. The prototype reflects the changes that
>> will
>> be brought in by
>>
>> https://bugzilla.yoctoproject.org/show_bug.cgi?id=7828
>>
>> But David is working on the state before that one. Sorry for the
>> confusion.
>>
>> Belén
>>
>> >
>> >
>> >>
>> >> Cheers
>> >>
>> >> Belén
>> >>
>> >>> - David
>> >>>
>> >>>> -----Original Message-----
>> >>>> From: toaster-bounces@yoctoproject.org [mailto:toaster-
>> >>>> bounces@yoctoproject.org] On Behalf Of Michael Wood
>> >>>> Sent: Monday, September 14, 2015 7:19 AM
>> >>>> To: toaster@yoctoproject.org
>> >>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>> >>>> missing when "IMAGE_FSTYPES" field is not properly edited
>> >>>>
>> >>>> On 02/09/15 16:58, Barros Pena, Belen wrote:
>> >>>>> On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com>
>> >>>> wrote:
>> >>>>>> Hi Belén,
>> >>>>>>
>> >>>>>> Thank you for the observation.
>> >>>>>>
>> >>>>>> I have updated "dreyna/project_fstypes_8126" to address that
>> issue,
>> >>>> and
>> >>>>>> the code now always pre-initializes the warning message element
>> so
>> >>>> that
>> >>>>>> the previous value is not dangling.
>> >>>>> yep: this seems to behave as expected now. Thanks!
>> >>>>>
>> >>>>> I've also realised that the base branch seems a bit old. Could
>> you
>> >>>> rebase
>> >>>>> and resubmit?
>> >>>>>
>> >>>>> Cheers
>> >>>>>
>> >>>>> Belén
>> >>>>>
>> >>>>>> - David
>> >>>>>>
>> >>>>>>
>> >>>>>>> -----Original Message-----
>> >>>>>>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>> >>>>>>> Sent: Wednesday, September 02, 2015 2:16 AM
>> >>>>>>> To: Reyna, David
>> >>>>>>> Cc: toaster@yoctoproject.org
>> >>>>>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages
>> are
>> >>>> missing
>> >>>>>>> when "IMAGE_FSTYPES" field is not properly edited
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
>> >>>>>>> On 02/09/2015 08:44, "Reyna, David"
>> <david.reyna@windriver.com>
>> >>>> wrote:
>> >>>>>>>> Hi Belén,
>> >>>>>>>>
>> >>>>>>>> Please find the patch for 8126 here:
>> >>>>>>>>
>> >>>>>>>>     dreyna/project_fstypes_8126
>> >>>>>>> Hi David,
>> >>>>>>>
>> >>>>>>> This is looking fairly good. I've only run across one problem.
>> >>>> This is
>> >>>>>>> how
>> >>>>>>> to reproduce:
>> >>>>>>>
>> >>>>>>> 1. Click the 'change' icon for IMAGE_FSTYPES
>> >>>>>>>
>> >>>>>>> 2. Deselect all values: the 'save' button becomes disabled and
>> the
>> >>>>>>> message
>> >>>>>>> asking you to select at least one image type appears. This is
>> the
>> >>>>>>> expected
>> >>>>>>> behaviour
>> >>>>>>>
>> >>>>>>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays
>> the
>> >>>> way
>> >>>>>>> it
>> >>>>>>> was before you clicked the 'change' icon. This is once more
>> the
>> >>>> correct
>> >>>>>>> behaviour
>> >>>>>>>
>> >>>>>>> 4. Now click the 'change' icon again. There are image types
>> >>>> selected,
>> >>>>>>> but
>> >>>>>>> the message 'You must select at least one image type' still
>> shows,
>> >>>> and
>> >>>>>>> the
>> >>>>>>> 'Save' button is disabled. This is not the correct behaviour.
>> As
>> >>>> long as
>> >>>>>>> there is at least one checkbox ticked you should see no
>> message
>> >>>> and the
>> >>>>>>> 'save' button should be enabled. If you make a change (untick
>> a
>> >>>> box),
>> >>>>>>> the
>> >>>>>>> validation kicks in and things return to the correct state.
>> Sounds
>> >>>> like
>> >>>>>>> we
>> >>>>>>> need to check the selected values whenever the 'change' icon
>> is
>> >>>> clicked
>> >>>>>>> Thanks!
>> >>>>>>>
>> >>>>>>> Belén
>> >>>>>>>
>> >>>>>>>> Note: for the message 'label' the I insert and then show when
>> >>>> there are
>> >>>>>>>> no matches, it is guaranteed not to pollute the database
>> because
>> >>>> it can
>> >>>>>>>> never be in the checked state.
>> >>>>>>>>
>> >>>>>>>> - David
>> >>>>>>>>
>> >>>> Some review comments below:
>> >>>>
>> >>>>   From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17
>> 00:00:00
>> >>>> 2001
>> >>>> From: David Reyna <David.Reyna@windriver.com>
>> >>>> Date: Wed, 2 Sep 2015 15:34:14 -0700
>> >>>> Subject: bitbake: toaster: display warnings for bad
>> "IMAGE_FSTYPES"
>> >>>> values
>> >>>>
>> >>>> Display warning message for IMAGE_FSTYPES when no value is
>> selected or
>> >>>> when the filter does not have any matches
>> >>>>
>> >>>> [YOCTO #8126]
>> >>>>
>> >>>> Signed-off-by: David Reyna <David.Reyna@windriver.com>
>> >>>>
>> >>>> diff --git
>> a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> >>>> b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> >>>> index 4c5a188..b477b4e 100644
>> >>>> --- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> >>>> +++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>> >>>> @@ -43,6 +43,7 @@
>> >>>>                        <input id="filter-image_fstypes"
>> type="text"
>> >>>> placeholder="Search image types" class="span4">
>> >>>>                        <div id="all-image_fstypes"
>> class="scrolling">
>> >>>>                        </div>
>> >>>> +                    <span class="help-block" id="fstypes-error-
>> >>>> message"></span>
>> >>>>                        <button id="apply-change-image_fstypes"
>> >>>> type="button" class="btn">Save</button>
>> >>>>                        <button id="cancel-change-image_fstypes"
>> >>>> type="button" class="btn btn-link">Cancel</button>
>> >>>>                    </form>
>> >>>> @@ -312,9 +313,11 @@
>> >>>>                });
>> >>>>                if ( 0 == any_checked ) {
>> >>>>
>> >>>>           $("#apply-change-
>> >>>> image_fstypes").attr("disabled","disabled");
>> >>>> +                $('#fstypes-error-message').html("You must
>> select at
>> >>>> least one image type");
>> >>>>
>> >>>> MW - Given that this error message doesn't change from this text,
>> >>>> rather than adding and removing the text add the text to the DOM
>> in
>> >>>> the #fstypes-error-message element and .hide .show this makes it
>> >>>> easier to update the error message in the html and keeps a
>> slightly
>> >>>> cleaner separation between the UI description and the logic of
>> the
>> >>>> javascript.
>> >>>>
>> >>>>       }
>> >>>>                else {
>> >>>>                    $("#apply-change-
>> >>>> image_fstypes").removeAttr("disabled");
>> >>>> +                $('#fstypes-error-message').html("");
>> >>>>                }
>> >>>>            }
>> >>>>
>> >>>> @@ -546,10 +549,14 @@
>> >>>>                    // Add the un-checked boxes second
>> >>>>                    for (var i = 0, length = fstypes_list.length;
>> i <
>> >>>> length; i++) {
>> >>>>                        if (0  > fstypes.indexOf("
>> >>>> "+fstypes_list[i].value+" ")) {
>> >>>> -                            html += '<label
>> class="checkbox"><input
>> >>>> type="checkbox" class="fs-checkbox-fstypes"
>> >>>>
>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>> >>>> ;
>> >>>> +                        html += '<label class="checkbox"><input
>> >>>> type="checkbox" class="fs-checkbox-fstypes"
>> >>>>
>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>> >>>> ;
>> >>>>                        }
>> >>>>                    }
>> >>>> +                // Add the 'no search matches' line last
>> >>>> +                html += '<label id="no-match-fstypes">No image
>> types
>> >>>> found</label>\n';
>> >>>>
>> >>>>
>> >>>> MW - Rather than appending this to the list just add the text to
>> the
>> >>>> #fstypes-error-message and set it it initially to
>> style="display:none"
>> >>>> and then toggle it with .hide .show
>> >>>>
>> >>>> +                // Display the list
>> >>>>                    document.getElementById("all-
>> >>>> image_fstypes").innerHTML = html;
>> >>>> +                $('#no-match-fstypes').hide();
>> >>>>
>> >>>>                    // Watch elements to disable Save when none
>> are
>> >>>> checked
>> >>>>                    $(".fs-checkbox-fstypes").each(function(){
>> >>>> @@ -558,8 +565,9 @@
>> >>>>                        });
>> >>>>                    });
>> >>>>
>> >>>> -                // clear the previous filter values
>> >>>> +                // clear the previous filter values and warning
>> >>>> messages
>> >>>>                    $("input#filter-image_fstypes").val("");
>> >>>> +                $('#fstypes-error-message').html("");
>> >>>>                });
>> >>>>
>> >>>>                $('#cancel-change-
>> image_fstypes').click(function(){
>> >>>> @@ -569,17 +577,24 @@
>> >>>>                });
>> >>>>
>> >>>>                $('#filter-image_fstypes').on('input', function(){
>> >>>> -                   var valThis = $(this).val().toLowerCase();
>> >>>> +                var valThis = $(this).val().toLowerCase();
>> >>>> +                var match_count=0;
>> >>>>
>> >>>>
>> >>>> MW - camelCase please
>> >>>>
>> >>>>
>> >>>>               $('#all-image_fstypes label').each(function(){
>> >>>>                        var text = $(this).text().toLowerCase();
>> >>>>                        var match = text.indexOf(valThis);
>> >>>>                        if (match >= 0) {
>> >>>>                            $(this).show();
>> >>>> +                        match_count += 1;
>> >>>>                        }
>> >>>>                        else {
>> >>>>                            $(this).hide();
>> >>>>                        }
>> >>>>                    });
>> >>>> +                if (0 == match_count) {
>> >>>>
>> >>>>
>> >>>> MW - This should be if (matchCount === 0)
>> >>>> triple equals for full equality and variable on the left
>> >>>>
>> >>>>
>> >>>>
>> >>>>    +                   $('#no-match-fstypes').show();
>> >>>> +                } else {
>> >>>> +                   $('#no-match-fstypes').hide();
>> >>>> +                }
>> >>>>                });
>> >>>>
>> >>>>                $('#apply-change-image_fstypes').click(function(){
>> >>>> --
>> >>>> cgit v0.10.2
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> _______________________________________________
>> >>>> toaster mailing list
>> >>>> toaster@yoctoproject.org
>> >>>> https://lists.yoctoproject.org/listinfo/toaster
>> >>> --
>> >>> _______________________________________________
>> >>> toaster mailing list
>> >>> toaster@yoctoproject.org
>> >>> https://lists.yoctoproject.org/listinfo/toaster
>> >
>>
>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
  2015-09-30 14:05                   ` Barros Pena, Belen
@ 2015-10-06 15:44                     ` Michael Wood
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Wood @ 2015-10-06 15:44 UTC (permalink / raw)
  To: Barros Pena, Belen, Reyna, David L (Wind River); +Cc: toaster@yoctoproject.org

Submitted upstream.

Thanks

Michael

On 30/09/15 15:05, Barros Pena, Belen wrote:
>
> On 30/09/2015 14:47, "Reyna, David" <david.reyna@windriver.com> wrote:
>
>> Hi Belén and Michael,
>>
>>> But David is working on the state before that one. Sorry for the
>>> confusion.
>> Is there an action for me?
> Not from the UI side of things. I am not sure about the code.
>
> Thanks!
>
> Belén
>
>> What I have now in the commit does work. I like the original design for
>> handling the "no options selected" in that (a) the message is clear and
>> in-context, and (b) the display does not jump.
>>
>> - David
>>
>>> -----Original Message-----
>>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>>> Sent: Wednesday, September 30, 2015 4:04 AM
>>> To: WOOD, MICHAEL; Reyna, David
>>> Cc: toaster@yoctoproject.org
>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>>> missing when "IMAGE_FSTYPES" field is not properly edited
>>>
>>>
>>>
>>> On 30/09/2015 11:59, "Michael Wood" <michael.g.wood@intel.com> wrote:
>>>
>>>> On 28/09/15 11:11, Barros Pena, Belen wrote:
>>>>> Sorry for the delay in looking at this.
>>>>>
>>>>> On 23/09/2015 10:02, "toaster-bounces@yoctoproject.org on behalf of
>>>>> Reyna,
>>>>> David" <toaster-bounces@yoctoproject.org on behalf of
>>>>> david.reyna@windriver.com> wrote:
>>>>>
>>>>>> Hi Michael,
>>>>>>
>>>>>> I have made the requested changes and updated
>>>>>> "dreyna/project_fstypes_8126", except for this one:
>>>>>>
>>>>>>> +                // Add the 'no search matches' line last
>>>>>>> +                html += '<label id="no-match-fstypes">No image
>>> types
>>>>>>> found</label>\n';
>>>>>>>
>>>>>>> MW - Rather than appending this to the list just add the text to
>>> the
>>>>>>> #fstypes-error-message
>>>>>>> and set it it initially to style="display:none" and then toggle
>>> it
>>>>>>> with
>>>>>>> .hide .show
>>>>>> Belén's design had this message appear within the selection list
>>> when
>>>>>> there were no search matches instead of in the optional error
>>> message
>>>>>> below it. That is why I did this implementation. I do toggle it
>>> with
>>>>>> .hide .show.
>>>>> FWIW, I agree with David on this one. The message has to be as
>>> close as
>>>>> possible to the context of the action. Having the 'no image types
>>> found'
>>>>> next to the buttons would be a bit too far away.
>>>>>
>>>>> The branch looks good from the UI standpoint.
>>>> Oh I was going on this design:
>>>> http://www.yoctoproject.org/toaster/localconf.html
>>> Ah, I see what you mean know. The prototype reflects the changes that
>>> will
>>> be brought in by
>>>
>>> https://bugzilla.yoctoproject.org/show_bug.cgi?id=7828
>>>
>>> But David is working on the state before that one. Sorry for the
>>> confusion.
>>>
>>> Belén
>>>
>>>>
>>>>> Cheers
>>>>>
>>>>> Belén
>>>>>
>>>>>> - David
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: toaster-bounces@yoctoproject.org [mailto:toaster-
>>>>>>> bounces@yoctoproject.org] On Behalf Of Michael Wood
>>>>>>> Sent: Monday, September 14, 2015 7:19 AM
>>>>>>> To: toaster@yoctoproject.org
>>>>>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages are
>>>>>>> missing when "IMAGE_FSTYPES" field is not properly edited
>>>>>>>
>>>>>>> On 02/09/15 16:58, Barros Pena, Belen wrote:
>>>>>>>> On 02/09/2015 15:46, "Reyna, David" <david.reyna@windriver.com>
>>>>>>> wrote:
>>>>>>>>> Hi Belén,
>>>>>>>>>
>>>>>>>>> Thank you for the observation.
>>>>>>>>>
>>>>>>>>> I have updated "dreyna/project_fstypes_8126" to address that
>>> issue,
>>>>>>> and
>>>>>>>>> the code now always pre-initializes the warning message element
>>> so
>>>>>>> that
>>>>>>>>> the previous value is not dangling.
>>>>>>>> yep: this seems to behave as expected now. Thanks!
>>>>>>>>
>>>>>>>> I've also realised that the base branch seems a bit old. Could
>>> you
>>>>>>> rebase
>>>>>>>> and resubmit?
>>>>>>>>
>>>>>>>> Cheers
>>>>>>>>
>>>>>>>> Belén
>>>>>>>>
>>>>>>>>> - David
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Barros Pena, Belen [mailto:belen.barros.pena@intel.com]
>>>>>>>>>> Sent: Wednesday, September 02, 2015 2:16 AM
>>>>>>>>>> To: Reyna, David
>>>>>>>>>> Cc: toaster@yoctoproject.org
>>>>>>>>>> Subject: Re: [Toaster] [review-request][PATCH] 8126 Messages
>>> are
>>>>>>> missing
>>>>>>>>>> when "IMAGE_FSTYPES" field is not properly edited
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 02/09/2015 08:44, "Reyna, David"
>>> <david.reyna@windriver.com>
>>>>>>> wrote:
>>>>>>>>>>> Hi Belén,
>>>>>>>>>>>
>>>>>>>>>>> Please find the patch for 8126 here:
>>>>>>>>>>>
>>>>>>>>>>>      dreyna/project_fstypes_8126
>>>>>>>>>> Hi David,
>>>>>>>>>>
>>>>>>>>>> This is looking fairly good. I've only run across one problem.
>>>>>>> This is
>>>>>>>>>> how
>>>>>>>>>> to reproduce:
>>>>>>>>>>
>>>>>>>>>> 1. Click the 'change' icon for IMAGE_FSTYPES
>>>>>>>>>>
>>>>>>>>>> 2. Deselect all values: the 'save' button becomes disabled and
>>> the
>>>>>>>>>> message
>>>>>>>>>> asking you to select at least one image type appears. This is
>>> the
>>>>>>>>>> expected
>>>>>>>>>> behaviour
>>>>>>>>>>
>>>>>>>>>> 3. Now, click the 'cancel' link. The IMAGE_FSTYPES value stays
>>> the
>>>>>>> way
>>>>>>>>>> it
>>>>>>>>>> was before you clicked the 'change' icon. This is once more
>>> the
>>>>>>> correct
>>>>>>>>>> behaviour
>>>>>>>>>>
>>>>>>>>>> 4. Now click the 'change' icon again. There are image types
>>>>>>> selected,
>>>>>>>>>> but
>>>>>>>>>> the message 'You must select at least one image type' still
>>> shows,
>>>>>>> and
>>>>>>>>>> the
>>>>>>>>>> 'Save' button is disabled. This is not the correct behaviour.
>>> As
>>>>>>> long as
>>>>>>>>>> there is at least one checkbox ticked you should see no
>>> message
>>>>>>> and the
>>>>>>>>>> 'save' button should be enabled. If you make a change (untick
>>> a
>>>>>>> box),
>>>>>>>>>> the
>>>>>>>>>> validation kicks in and things return to the correct state.
>>> Sounds
>>>>>>> like
>>>>>>>>>> we
>>>>>>>>>> need to check the selected values whenever the 'change' icon
>>> is
>>>>>>> clicked
>>>>>>>>>> Thanks!
>>>>>>>>>>
>>>>>>>>>> Belén
>>>>>>>>>>
>>>>>>>>>>> Note: for the message 'label' the I insert and then show when
>>>>>>> there are
>>>>>>>>>>> no matches, it is guaranteed not to pollute the database
>>> because
>>>>>>> it can
>>>>>>>>>>> never be in the checked state.
>>>>>>>>>>>
>>>>>>>>>>> - David
>>>>>>>>>>>
>>>>>>> Some review comments below:
>>>>>>>
>>>>>>>    From bac104e20efc46d4746af58aebb0f920c37edfc3 Mon Sep 17
>>> 00:00:00
>>>>>>> 2001
>>>>>>> From: David Reyna <David.Reyna@windriver.com>
>>>>>>> Date: Wed, 2 Sep 2015 15:34:14 -0700
>>>>>>> Subject: bitbake: toaster: display warnings for bad
>>> "IMAGE_FSTYPES"
>>>>>>> values
>>>>>>>
>>>>>>> Display warning message for IMAGE_FSTYPES when no value is
>>> selected or
>>>>>>> when the filter does not have any matches
>>>>>>>
>>>>>>> [YOCTO #8126]
>>>>>>>
>>>>>>> Signed-off-by: David Reyna <David.Reyna@windriver.com>
>>>>>>>
>>>>>>> diff --git
>>> a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>>>>> b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>>>>> index 4c5a188..b477b4e 100644
>>>>>>> --- a/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>>>>> +++ b/bitbake/lib/toaster/toastergui/templates/projectconf.html
>>>>>>> @@ -43,6 +43,7 @@
>>>>>>>                         <input id="filter-image_fstypes"
>>> type="text"
>>>>>>> placeholder="Search image types" class="span4">
>>>>>>>                         <div id="all-image_fstypes"
>>> class="scrolling">
>>>>>>>                         </div>
>>>>>>> +                    <span class="help-block" id="fstypes-error-
>>>>>>> message"></span>
>>>>>>>                         <button id="apply-change-image_fstypes"
>>>>>>> type="button" class="btn">Save</button>
>>>>>>>                         <button id="cancel-change-image_fstypes"
>>>>>>> type="button" class="btn btn-link">Cancel</button>
>>>>>>>                     </form>
>>>>>>> @@ -312,9 +313,11 @@
>>>>>>>                 });
>>>>>>>                 if ( 0 == any_checked ) {
>>>>>>>
>>>>>>>            $("#apply-change-
>>>>>>> image_fstypes").attr("disabled","disabled");
>>>>>>> +                $('#fstypes-error-message').html("You must
>>> select at
>>>>>>> least one image type");
>>>>>>>
>>>>>>> MW - Given that this error message doesn't change from this text,
>>>>>>> rather than adding and removing the text add the text to the DOM
>>> in
>>>>>>> the #fstypes-error-message element and .hide .show this makes it
>>>>>>> easier to update the error message in the html and keeps a
>>> slightly
>>>>>>> cleaner separation between the UI description and the logic of
>>> the
>>>>>>> javascript.
>>>>>>>
>>>>>>>        }
>>>>>>>                 else {
>>>>>>>                     $("#apply-change-
>>>>>>> image_fstypes").removeAttr("disabled");
>>>>>>> +                $('#fstypes-error-message').html("");
>>>>>>>                 }
>>>>>>>             }
>>>>>>>
>>>>>>> @@ -546,10 +549,14 @@
>>>>>>>                     // Add the un-checked boxes second
>>>>>>>                     for (var i = 0, length = fstypes_list.length;
>>> i <
>>>>>>> length; i++) {
>>>>>>>                         if (0  > fstypes.indexOf("
>>>>>>> "+fstypes_list[i].value+" ")) {
>>>>>>> -                            html += '<label
>>> class="checkbox"><input
>>>>>>> type="checkbox" class="fs-checkbox-fstypes"
>>>>>>>
>>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>>>>>>> ;
>>>>>>> +                        html += '<label class="checkbox"><input
>>>>>>> type="checkbox" class="fs-checkbox-fstypes"
>>>>>>>
>>> value="'+fstypes_list[i].value+'">'+fstypes_list[i].value+'</label>\n'
>>>>>>> ;
>>>>>>>                         }
>>>>>>>                     }
>>>>>>> +                // Add the 'no search matches' line last
>>>>>>> +                html += '<label id="no-match-fstypes">No image
>>> types
>>>>>>> found</label>\n';
>>>>>>>
>>>>>>>
>>>>>>> MW - Rather than appending this to the list just add the text to
>>> the
>>>>>>> #fstypes-error-message and set it it initially to
>>> style="display:none"
>>>>>>> and then toggle it with .hide .show
>>>>>>>
>>>>>>> +                // Display the list
>>>>>>>                     document.getElementById("all-
>>>>>>> image_fstypes").innerHTML = html;
>>>>>>> +                $('#no-match-fstypes').hide();
>>>>>>>
>>>>>>>                     // Watch elements to disable Save when none
>>> are
>>>>>>> checked
>>>>>>>                     $(".fs-checkbox-fstypes").each(function(){
>>>>>>> @@ -558,8 +565,9 @@
>>>>>>>                         });
>>>>>>>                     });
>>>>>>>
>>>>>>> -                // clear the previous filter values
>>>>>>> +                // clear the previous filter values and warning
>>>>>>> messages
>>>>>>>                     $("input#filter-image_fstypes").val("");
>>>>>>> +                $('#fstypes-error-message').html("");
>>>>>>>                 });
>>>>>>>
>>>>>>>                 $('#cancel-change-
>>> image_fstypes').click(function(){
>>>>>>> @@ -569,17 +577,24 @@
>>>>>>>                 });
>>>>>>>
>>>>>>>                 $('#filter-image_fstypes').on('input', function(){
>>>>>>> -                   var valThis = $(this).val().toLowerCase();
>>>>>>> +                var valThis = $(this).val().toLowerCase();
>>>>>>> +                var match_count=0;
>>>>>>>
>>>>>>>
>>>>>>> MW - camelCase please
>>>>>>>
>>>>>>>
>>>>>>>                $('#all-image_fstypes label').each(function(){
>>>>>>>                         var text = $(this).text().toLowerCase();
>>>>>>>                         var match = text.indexOf(valThis);
>>>>>>>                         if (match >= 0) {
>>>>>>>                             $(this).show();
>>>>>>> +                        match_count += 1;
>>>>>>>                         }
>>>>>>>                         else {
>>>>>>>                             $(this).hide();
>>>>>>>                         }
>>>>>>>                     });
>>>>>>> +                if (0 == match_count) {
>>>>>>>
>>>>>>>
>>>>>>> MW - This should be if (matchCount === 0)
>>>>>>> triple equals for full equality and variable on the left
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>     +                   $('#no-match-fstypes').show();
>>>>>>> +                } else {
>>>>>>> +                   $('#no-match-fstypes').hide();
>>>>>>> +                }
>>>>>>>                 });
>>>>>>>
>>>>>>>                 $('#apply-change-image_fstypes').click(function(){
>>>>>>> --
>>>>>>> cgit v0.10.2
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> _______________________________________________
>>>>>>> toaster mailing list
>>>>>>> toaster@yoctoproject.org
>>>>>>> https://lists.yoctoproject.org/listinfo/toaster
>>>>>> --
>>>>>> _______________________________________________
>>>>>> toaster mailing list
>>>>>> toaster@yoctoproject.org
>>>>>> https://lists.yoctoproject.org/listinfo/toaster



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-10-06 15:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-02  7:44 [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited Reyna, David
2015-09-02  9:16 ` Barros Pena, Belen
2015-09-02 14:46   ` Reyna, David
2015-09-02 15:58     ` Barros Pena, Belen
2015-09-14 14:19       ` Michael Wood
2015-09-23  9:02         ` Reyna, David
2015-09-28 10:11           ` Barros Pena, Belen
2015-09-30 10:59             ` Michael Wood
2015-09-30 11:04               ` Barros Pena, Belen
2015-09-30 13:47                 ` Reyna, David
2015-09-30 14:05                   ` Barros Pena, Belen
2015-10-06 15:44                     ` Michael Wood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.