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: [email protected] [mailto:toaster- > [email protected]] On Behalf Of Michael Wood > Sent: Monday, September 14, 2015 7:19 AM > To: [email protected] > 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" <[email protected]> > 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:[email protected]] > >>> Sent: Wednesday, September 02, 2015 2:16 AM > >>> To: Reyna, David > >>> Cc: [email protected] > >>> 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" <[email protected]> > 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 <[email protected]> > 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 <[email protected]> > > 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 > [email protected] > https://lists.yoctoproject.org/listinfo/toaster -- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
