* Re: [review-request] michaelw/toaster/layer-details
2014-12-19 13:08 ` Barros Pena, Belen
@ 2015-01-12 18:24 ` Michael Wood
0 siblings, 0 replies; 3+ messages in thread
From: Michael Wood @ 2015-01-12 18:24 UTC (permalink / raw)
To: Barros Pena, Belen; +Cc: toaster@yoctoproject.org
Hi,
Thanks for the review. I've pushed a bunch of WIP commits on top of this
branch. Probably need some discussion on some items that aren't
currently resolved and I haven't found a way to do the added/remove
notifications yet.
On 19/12/14 13:08, Barros Pena, Belen wrote:
> On 18/12/2014 23:55, "Michael Wood"<michael.g.wood@intel.com> wrote:
>
>> Review request for poky-contrib michaelw/toaster/layer-details
> Hi Michael,
>
> Thanks for this. Some comments about the interface:
>
> * The breadcrumb is missing the 'all layers' item and the name of the
> current layer. It says:
>
> all builds > project_name > Layer Details
>
> But is should say:
>
> all builds > project_name > All layers > layer_name (revision)
Done
> * The layout is a bit messed up: the heading is not aligned properly. The
> structure should be:
>
> <div class="container-fluid top-padded">
> <div class="row-fluid">
> <div class="section">
> <!-- the breadcrumb goes here -->
> </div>
> <div class="span11">
> <!-- the page heading goes here -->
> </div>
> <div class="span7">
> <!-- the tabs and the tab content go here -->
> </div>
> <div class="span4 well">
> <!-- this is the right column -->
> </div>
> <div>
> </div>
It's not possible to get this exact structure because part of the
structure is inherited from the common base template.
The closest layout to the mock ups is
container-fluid top-padded
row-fluid
span11
page-header
row-fluid
span7
span4
> * When the page heading shows the first 10 characters of the commit SHA,
> we are missing the tooltip on hover that shows you the complete SHA (see
> page 9 of the design document)
Done
> NOT EDITABLE VERSION
>
> * Commit SHA is showing instead of the branch name. For example, if you
> select a layer that has a branch name in the 'branch, tag or commit'
> column in the all layers page, you will see that the layer details page is
> showing a commit in the page heading, instead of the branch name.
Depends on Bug #7031
https://bugzilla.yoctoproject.org/show_bug.cgi?id=7031
> * In the right hand column, the line height inside the <dd> items is not
> quite right. It seems to be 25px instead of 20px, although I can see that
> it is set to 20px in default.css. If I change the span tag to a p tag, the
> line height is the correct one.
In default.css child items of the dd,dt are line-height: 25px apart
from p tag this is toaster wide and not to do with the layer-details
page so not sure how to proceed here.
> * In the layer details tab, the Yocto Project compatibility list item can
> go.
I'll remove this as a final commit on top of the current work so it's a
self contained commit
> * In the layer details tab, the "Last commit" information is missing
We currently don't have this data available to display
> * There is something funky going on with the information in the layer
> details tab. It might be related to empty information. If you select the
> meta-fsl-arm-extra layer, for example, that layer doesn't seem to have a
> subdirectory, so all other items of information shift up, so you see a
> commit next to the 'repository subdirectory' label, and the layer
> dependencies split between the Yocto Project compatibility and the
> Dependencies items. If a bit of information is empty, we remove the whole
> list item (both dt and dd tags), so if a layer does not have a
> subdirectory, we don't who the <dt>Subdirectory<dt> element either.
Yes empty information is handled in the dl element by just shifting up
to the next dt that has data.
This is a known issue
http://stackoverflow.com/questions/12883527/why-is-the-display-of-labels-and-data-not-aligned-using-twitter-bootstrap-2-if-t
I've added a fix to check if any dd in a dt dd pair has no data in or
the edit form is not visible to hide the pair
> * The 'Git repository URL' and the 'Subdirectory' values are missing the
> external links (see page 15 of the design document)
Done
> * In the 'Targets' tab, when the layer is not added to the project and the
> number of targets is > 0, the big button above the table should say: "Add
> the <layer_name> layer to your project to enable these targets", and all
> the 'Build target' buttons in the table should be inactive
OK but be aware that this kind of thing costs complexity. It's actually
"on switching tab to targets and targets is > 0 and layer is not in
project then change button text to longer version then On switching to
the layer details tab if the layer is not in the project change text to
the default text and on switching to the machines tab if the layer is
not in the project change text back to stored default text. "
> * The targets table should be sorted by target name in ascending
> alphabetical order
Done
> * In the targets table, the external link to the layer index next to the
> target name is missing (see page 21 of the design document)
Done
> * In tables with more than 10 rows, the search is missing (see pages 25 to
> 27 of the design document)
Done
> * In the targets tables with more than 10 rows, the number of entries
> displayed at the bottom of the table can go, the pagination should be
> left-aligned with the table and horizontally aligned to the show rows
> control. You can see what I mean at
> http://www.yoctoproject.org/toaster/layer-details-with-search.html
Done
> * In the not editable version of the page, when the targets table is
> empty, the notification says: "The <layer_name> layer does not provide any
> targets". It is likely that the information coming from the layer source
> is correct, and there will never be any targets there.
Hmm don't understand what you mean here
> * In the 'Machines' tab, when the layer is not added to the project and
> the number of machines is > 0, the big button above the table should say:
> "Add the <layer_name> layer to your project to enable these machines", and
> all the 'Select machine' buttons in the table should be inactive
Done
> * The help bubble next to the "Select machine" table heading can go
Done
> * The machines table should be sorted by machine name in ascending
> alphabetical order
Done
> * The machines table is missing the external link to the machine file (see
> page 23 of the design document)
We don't currently have this information available
> * In the machine tables with more than 10 items, we are missing the
> search, show rows and pagination controls
Done
> * In the not editable version of the page, when the machines table is
> empty, the notification says: "The <layer_name> layer does not provide any
> machines". It is likely that the information coming from the layer source
> is correct, and there will never be any targets there.
Not sure what you mean here
> * When I add / delete a layer to the project from this page, the tooltip
> and the notification are missing (see pages 28 to 30 of the design
> document)
Not yet work out a way to do this, also the tooltip thing I thought was
a bug!
> * When I click the 'Select machine' buttons, nothing happens
Done
> * Sometimes, the add layer button does nothing, other times it works. I am
> not sure, but it seems to work when the layer you are trying to add has
> unsatisfied dependencies, but not when the layer doesn't have any
> unsatisfied dependencies.
Hmm I can't recreate this problem
> EDITABLE VERSION
>
> * When a field is empty, it should say "not set" instead of "none" and
> should have the .muted class applied
Done
> * In the editable state, the button labels should be 'save' and 'cancel',
> instead of 'change' and 'cancel'. I know this sounds crazy, but there is a
> reason why I set this to 'save'. I don't quite remember right now, but
> probably so that it makes sense in all contexts.
Done
> * If a value is empty, when you click on the 'change' icon, the value of
> the input field is set to 'none'. In this case, the input field should be
> empty
Done I think
> * When you are editing something, while the input field is empty, the
> 'save' button should be disabled. This is important because it is the way
> we make sure you don't set mandatory values, like the 'Git repo URL' to
> blank (something you can do right now).
Done
> * Non-mandatory items like Summary and Description should have a 'delete'
> icon whenever they have a value (see page 13 of the design document).
Done
> * In the right-hand column, the edit forms should have 20px of bottom
> margin, to separate them a bit from the rest of the definition list. You
> can see it athttp://yoctoproject.org/toaster/layer-details-imported.html
Done I think
> * the default.css file has 2 dd form declarations, one after the other. We
> should not have the 2 of them
I don't think this is a layer details page thing, please file a bug to
track this one
> * In the layer details tab, we need to add 10px of top margin to the <dt>
> element when you click the 'edit' button to make sure it correctly aligns
> horizontally with the form. You can check it out here
> http://yoctoproject.org/toaster/layer-details-imported.html
>
>
Done
>> commit 7f6b4a2f2d9b1e83593335be09801f7b2efa812a
>> Author: Michael Wood<michael.g.wood@intel.com>
>> Date: Thu Dec 11 18:32:54 2014 +0000
>>
>> bitbake: toaster: Add layer details page feature
>>
>> This commit adds the layer details page which shows the metadata
>> for the
>> layer such as layer description, machines associated with the layer
>> as well
>> as the targets provided.
>> If the layer is an imported layer this page also allows you to update
>> the layer's configuration.
>> From this page you can add/remove the layer from the current project
>>
>> Signed-off-by: Michael Wood<michael.g.wood@intel.com>
>>
>>
>> Thanks,
>>
>> Michael
>> --
>> _______________________________________________
>> toaster mailing list
>> toaster@yoctoproject.org
>> https://lists.yoctoproject.org/listinfo/toaster
^ permalink raw reply [flat|nested] 3+ messages in thread