[Toaster] [review-request][PATCH] 8126 Messages are missing when "IMAGE_FSTYPES" field is not properly edited
Reyna, David
david.reyna at windriver.com
Wed Sep 30 06:47:30 PDT 2015
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: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