[Toaster] [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
Barros Pena, Belen
belen.barros.pena at intel.com
Wed Sep 30 07:05:09 PDT 2015
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