[Toaster] [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
Michael Wood
michael.g.wood at intel.com
Wed Sep 30 03:59:38 PDT 2015
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 at yoctoproject.org on behalf of Reyna,
> David" <toaster-bounces at yoctoproject.org on behalf of
> david.reyna at 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 at yoctoproject.org [mailto:toaster-
>>> bounces at yoctoproject.org] On Behalf Of Michael Wood
>>> Sent: Monday, September 14, 2015 7:19 AM
>>> To: toaster at 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 at 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 at intel.com]
>>>>>> Sent: Wednesday, September 02, 2015 2:16 AM
>>>>>> To: Reyna, David
>>>>>> Cc: toaster at 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 at 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 at 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 at 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 at yoctoproject.org
>>> https://lists.yoctoproject.org/listinfo/toaster
>> --
>> _______________________________________________
>> toaster mailing list
>> toaster at yoctoproject.org
>> https://lists.yoctoproject.org/listinfo/toaster
More information about the toaster
mailing list