On 30/09/2015 14:47, "Reyna, David" <[email protected]> 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:[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
