[Toaster] [review-request] michaelw/toaster-frontend-cleanups
Damian, Alexandru
alexandru.damian at intel.com
Fri May 1 09:30:02 PDT 2015
All taken for submission,
Thank you,
Alex
On Fri, Apr 24, 2015 at 5:06 PM, Barros Pena, Belen <
belen.barros.pena at intel.com> wrote:
> On 23/04/2015 16:08, "Michael Wood" <michael.g.wood at intel.com> wrote:
>
> >New version pushed with modified in-cell notification animation as we
> >discussed offline.
> >
> >If this is OK let's get this in the submission queue.
>
> This looks good to me.
>
> Thanks!
>
> Belén
>
> >
> >Michael
> >
> >On 21/04/15 20:50, Michael Wood wrote:
> >> Thanks
> >>
> >> On 21/04/15 18:36, Barros Pena, Belen wrote:
> >>> On 21/04/2015 13:34, "Michael Wood" <michael.g.wood at intel.com> wrote:
> >>>
> >>>> Version 2 pushed with more refactoring.
> >>> Hi Michael,
> >>>
> >>> This is looking pretty good. My list of issues is pretty small. Here it
> >>> comes:
> >>>
> >>> * In the add and delete layer notifications, the names of the added
> >>> layers
> >>> should be links to the corresponding layer details pages
> >> Fixed pushed
> >>> * Add layer notification when adding more than one layer due to
> >>> dependencies, in the 'all compatible layers' page: the notification
> >>> in the
> >>> 'all compatible layers' page is different to the one in the project
> >>> page.
> >>> My preferred version in the one in the 'all compatible layers' page. I
> >>> wonder if it's possible to change the project page, although I am ok
> >>> with
> >>> doing this at a later stage.
> >>
> >> The issue with the project page is that it generates the notification
> >> by looking at the difference between "the list of layers before you
> >> added them" and "the list now" and then that difference is the list of
> >> layers added, which means it doesn't know /why/ the layers were added,
> >> so it's not possible to say which layer was added by the user and
> >> which were added by the system as dependencies.
> >>
> >> We could look to see if the user added the layers in the project page
> >> and then try and filter the resulting layers added list, the problem
> >> then would be what happens when you add layers outside of the project
> >> page, it would show a notification but would be unable to handle it in
> >> the correct way.
> >>
> >> tldr; It would be difficult to change this as it's ingrained in how
> >> the page works, if a updated design is in the pipeline it would be
> >> better to wait.
> >>
> >>
> >>>
> >>> * Add layer notification in the 'all compatible recipes' page: the
> >>> notification is missing the name of the layer selected by the user. It
> >>> only shows the name of the dependencies.
> >> Fixed pushed
> >>>
> >>> * In the 'all compatible recipes' and 'all compatible machines' pages,
> >>> when you add a layer, the cell tooltip showing the number of layers
> >>> added
> >>> shows up for all the recipes provided by the layer I am adding. I know
> >>> that's the way it works in the 'all compatible layers' page, but in
> >>>that
> >>> context you have gone through the layer dependencies dialog, so it
> >>> kind of
> >>> makes sense. But in the context of the 'all compatible recipes' page,
> >>>it
> >>> looks quite strange. Ideally in this page and the 'all compatible
> >>> machines' page the cell tooltip would only show in the cell you have
> >>> clicked (although the buttons do change for all the recipes provided by
> >>> the added layer). This is the way it currently works in master, if you
> >>> want to have a look at the difference. An alternative would be a
> >>> different
> >>> type of notification: one that displays fixed-positioned at the top
> >>> of the
> >>> viewport, and not at the top of the page, and therefore it's always
> >>> visible to users. This would allow us to get rid of the cell
> >>> tooltips, and
> >>> just exchange the buttons.
> >>
> >> Yeah, the alternative with the fixed position notification would be
> >> possible, the special casing of the button wouldn't be however.
> >>
> >> Personally I find the cell tooltip thing quite nice in all the pages
> >> because the layer-add behaviour is consistent, doesn't distract me
> >> away from the table content and shows the buttons changing which I
> >> don't notice otherwise.
> >>
> >>
> >>>
> >>> Thanks!
> >>>
> >>> Belén
> >>>
> >>>
> >>>> I found out there were a few more places which had custom code for
> >>>> handling add/remove layer which was causing the differing behaviour.
> >>>> Also fixed the heading, a typo in the help text a number of jshint
> >>>> issues and the inline notifications for added layers.
> >>>>
> >>>> Summary: 18 files changed, 439 insertions(+), 731 deletions(-)
> >>>>
> >>>>
> >>>> .../lib/toaster/toastergui/static/css/default.css | 2 +-
> >>>> .../toastergui/static/html/layer_deps_modal.html | 17 ++
> >>>> bitbake/lib/toaster/toastergui/static/js/base.js | 39 ++--
> >>>> .../toaster/toastergui/static/js/importlayer.js | 20 +-
> >>>> .../lib/toaster/toastergui/static/js/layerBtn.js | 71 +++++++
> >>>> .../toaster/toastergui/static/js/layerDepsModal.js | 90 +++++++++
> >>>> .../toaster/toastergui/static/js/layerdetails.js | 99 ++-------
> >>>> .../lib/toaster/toastergui/static/js/libtoaster.js | 95 ++++++++-
> >>>> .../lib/toaster/toastergui/static/js/machines.js | 97 ---------
> >>>> .../lib/toaster/toastergui/static/js/projectapp.js | 31 ++-
> >>>> bitbake/lib/toaster/toastergui/templates/base.html | 19 +-
> >>>> .../toaster/toastergui/templates/importlayer.html | 6 +-
> >>>> .../toaster/toastergui/templates/layerdetails.html | 9 +-
> >>>> .../lib/toaster/toastergui/templates/layers.html | 224
> >>>> +++------------------
> >>>> .../toastergui/templates/layers_dep_modal.html | 99 ---------
> >>>> .../lib/toaster/toastergui/templates/machines.html | 35 ++--
> >>>> .../lib/toaster/toastergui/templates/targets.html | 215
> >>>> ++++----------------
> >>>> bitbake/lib/toaster/toastergui/views.py | 2 +-
> >>>>
> >>>>
> >>>> commit a4ed04911d428944824f5b20f9ee642fdf5feea2
> >>>> Author: Michael Wood <michael.g.wood at intel.com>
> >>>> Date: Tue Apr 21 11:59:37 2015 +0100
> >>>>
> >>>> toaster: bitbake: Refactor and expand layer add remove mechanism
> >>>>
> >>>> We have multiple pages which have buttons to add and remove
> >>>>layers
> >>>> this
> >>>> patch adds functionality to libtoaster to abstract this and
> >>>> implements
> >>>> it in the pages affected. We handle loading and showing the
> >>>> dependencies
> >>>> dialog here too and generating the notification messages.
> >>>> Also implemented is using the selectmachine api from the
> >>>> projectapp
> >>>> to
> >>>> avoid having to handle this in each page that allows selecting
> >>>> machines.
> >>>> A small number of jshint issues, help text and the machine page
> >>>> name
> >>>> have also been fixed.
> >>>>
> >>>> Signed-off-by: Michael Wood <michael.g.wood at intel.com>
> >>>>
> >>>> commit 507fd79963318e7788aef057fe5f1c699032dd14
> >>>> Author: Michael Wood <michael.g.wood at intel.com>
> >>>> Date: Tue Apr 21 11:38:03 2015 +0100
> >>>>
> >>>> bitbake: toaster: projectapp Implement machine select command
> >>>>
> >>>> Use the project page to select the machine rather than setting
> >>>> it and
> >>>> then redirecting to the project page. This will also avoid having
> >>>> to have a
> >>>> special handler in the machines page it's self.
> >>>>
> >>>> Signed-off-by: Michael Wood <michael.g.wood at intel.com>
> >>>>
> >>>> commit 927c40e686736f70a0557f93bd42009dfbdefae8
> >>>> Author: Michael Wood <michael.g.wood at intel.com>
> >>>> Date: Fri Apr 10 18:15:03 2015 +0100
> >>>>
> >>>> bitbake: toaster: Move project context variables to common scope
> >>>>
> >>>> We have a bunch of context data which are used in multiple
> >>>> pages so
> >>>> it
> >>>> makes more sense to have this in a single place libtoaster.ctx
> >>>> that's
> >>>> accessible from each page rather than request it from every page.
> >>>>
> >>>> Signed-off-by: Michael Wood <michael.g.wood at intel.com>
> >>>>
> >>>>
> >>>> Michael
> >>>>
> >>>> On 16/04/15 15:11, Barros Pena, Belen wrote:
> >>>>> Hi Michael,
> >>>>>
> >>>>> On 14/04/2015 15:22, "Michael Wood" <michael.g.wood at intel.com>
> wrote:
> >>>>>
> >>>>>> poky-contrib michaelw/toaster-frontend-cleanups
> >>>>> I've checked what might be a v2 of this branch
> >>>>> (fd14503957379243e6ec34aea5f92b79dd2aefa8), and I have some comments:
> >>>>>
> >>>>> * Can we change the plural case in the notification show in the 'all
> >>>>> compatible' pages to match the one we use in the project page? The
> >>>>> 'all
> >>>>> compatible' pages are using 'layer(s)' and the 'project' page says
> >>>>> 'layers'. In the first, the '(s)' bit is not really needed because
> >>>>>the
> >>>>> word appears in singular when you only add one layer. Screenshots
> >>>>> attached
> >>>>>
> >>>>> * The add layer and delete layer buttons are not working in the 'all
> >>>>> compatible layers' page, in the 'all compatible machines' page and in
> >>>>> the
> >>>>> layer details page
> >>>>>
> >>>>> * The 'edit columns' menu seems to be broken in the 'all compatible
> >>>>> machines' page. All columns are shown by default, but when you
> >>>>> check the
> >>>>> 'edit columns' menu content some of the checkboxes are not selected
> >>>>>
> >>>>> * The h1 in the 'all compatible machines' page says 'all machines'
> >>>>> instead
> >>>>> of 'all compatible machines'
> >>>>>
> >>>>> Thanks!
> >>>>>
> >>>>> Belén
> >>>>>
> >>>>>
> >>>>>
> >>>>>> commit 310409d14c1af131ac39121936a3bb66741c39dc
> >>>>>> Author: Michael Wood <michael.g.wood at intel.com>
> >>>>>> Date: Tue Apr 14 15:12:35 2015 +0100
> >>>>>>
> >>>>>> bitbake: toaster: Create and use a single common layer
> >>>>>> add/remove
> >>>>>> mechanism
> >>>>>>
> >>>>>> This adds a common add/remove layer mechanism that will also
> >>>>>> handle
> >>>>>> showing
> >>>>>> the modal dialog if there are dependencies for the layer which
> >>>>>> also
> >>>>>> need
> >>>>>> adding. To help bring consistency this also adds a layer
> >>>>>> added/removed
> >>>>>> alert utility function to generate the alert message. This
> >>>>>> touches
> >>>>>> all
> >>>>>> places where layers can be added or removed to port them to
> >>>>>> this
> >>>>>> new
> >>>>>> common mechanism.
> >>>>>>
> >>>>>> Signed-off-by: Michael Wood <michael.g.wood at intel.com>
> >>>>>>
> >>>>>> commit c1c7a3435dcb70d82c2f9c710140c038cc9d205b
> >>>>>> Author: Michael Wood <michael.g.wood at intel.com>
> >>>>>> Date: Fri Apr 10 18:15:03 2015 +0100
> >>>>>>
> >>>>>> bitbake: toaster: Move project context variables to common
> >>>>>> scope
> >>>>>>
> >>>>>> We have a bunch of context data which are used in multiple
> >>>>>> pages
> >>>>>> so
> >>>>>> it
> >>>>>> makes more sense to have this in a single place libtoaster.ctx
> >>>>>> that's
> >>>>>> accessible from each page rather than request it from every
> >>>>>> page.
> >>>>>>
> >>>>>> Signed-off-by: Michael Wood <michael.g.wood at intel.com>
> >>>>>>
> >>>>>> --
> >>>>>> _______________________________________________
> >>>>>> 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
>
--
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150501/58a5dafb/attachment-0001.html>
More information about the toaster
mailing list