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:[email protected]] > Sent: Wednesday, September 30, 2015 4:04 AM > To: WOOD, MICHAEL; Reyna, David > Cc: [email protected] > 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" <[email protected]> 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, "[email protected] on behalf of > >>Reyna, > >> David" <[email protected] on behalf of > >> [email protected]> 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: [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 > > > -- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
