[Toaster] [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
Michael Wood
michael.g.wood at intel.com
Tue Oct 6 08:44:35 PDT 2015
Submitted upstream.
Thanks
Michael
On 30/09/15 15:05, Barros Pena, Belen wrote:
>
> On 30/09/2015 14:47, "Reyna, David" <david.reyna at 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 at intel.com]
>>> Sent: Wednesday, September 30, 2015 4:04 AM
>>> To: WOOD, MICHAEL; 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 30/09/2015 11:59, "Michael Wood" <michael.g.wood at 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 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
>>> 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 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