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

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

[Michael Wood]

poky-contrib michaelw/toaster-frontend-cleanups


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>

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

[Barros Pena, Belen]

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

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


[-- Attachment #2: project-page.png --]
[-- Type: image/png, Size: 176070 bytes --]

[-- Attachment #3: all-recipes-page.png --]
[-- Type: image/png, Size: 201167 bytes --]

[-- Attachment #4: project-page-one-layer.png --]
[-- Type: image/png, Size: 139373 bytes --]

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

[Michael Wood]

Version 2 pushed with more refactoring.

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

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

[Barros Pena, Belen]

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

* 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.

* 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.

* 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.

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
>

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

[Michael Wood]

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

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

[Michael Wood]

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
>

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

[Barros Pena, Belen]

On 23/04/2015 16:08, "Michael Wood" <michael.g.wood@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@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
>>
>

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

[Damian, Alexandru]

[-- Attachment #1: Type: text/plain, Size: 11825 bytes --]

All taken for submission,

Thank you,
Alex

On Fri, Apr 24, 2015 at 5:06 PM, Barros Pena, Belen <
belen.barros.pena@intel.com> wrote:

> On 23/04/2015 16:08, "Michael Wood" <michael.g.wood@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@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
> >>
> >
>
> --
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>



-- 
Alex Damian
Yocto Project
SSG / OTC

[-- Attachment #2: Type: text/html, Size: 17784 bytes --]