Re: [review-request] michaelw/toaster-frontend-cleanups

[Michael Wood <michael.g.wood@intel.com>]

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.

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@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@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@intel.com>
>>>
>>> commit 507fd79963318e7788aef057fe5f1c699032dd14
>>> Author: Michael Wood <michael.g.wood@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@intel.com>
>>>
>>> commit 927c40e686736f70a0557f93bd42009dfbdefae8
>>> Author: Michael Wood <michael.g.wood@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@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@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@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@intel.com>
>>>>>
>>>>> commit c1c7a3435dcb70d82c2f9c710140c038cc9d205b
>>>>> Author: Michael Wood <michael.g.wood@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@intel.com>
>>>>>
>>>>> -- 
>>>>> _______________________________________________
>>>>> toaster mailing list
>>>>> toaster@yoctoproject.org
>>>>> https://lists.yoctoproject.org/listinfo/toaster
>