Code review comment for lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/navi

Revision history for this message
David Planella (dpm) wrote :

Nice one, looking forward to seeing navigation!

A few things I've noticed:

1) There are some changes required to the docs done by the documentation/generate_html.sh script. However, I believe this script was put there to manually build the documentation for testing purposes. Thus it is not called during the package build and these changes will not be applied, if I understand it correctly:

51 +sed -r -i 's@("main-content">)@\1<ul class="breadcrumb">@g' html/*[a-z].html

2) When running the ./generate_html.sh script locally I get this warning:

$ ./generate_html.sh
/tmp/navi/modules/Ubuntu/Components/overview.qdoc:17: warning: Can't link to 'All Modules'

3) The tables on the home page should not be full width, there seems to be a regression on the layout of the home page. I.e.:

In this branch:
  http://people.canonical.com/~dpm/sdk-docs/html/ubuntu-module.html

Expected:
  http://developer.ubuntu.com/api/ubuntu-12.10/qml/mobile/overview-ubuntu-sdk.html

4) There are no breadcrumbs on the "Basic QML Types" page, and thus no way to go back to the home page [1]

  http://people.canonical.com/~dpm/sdk-docs/html/qmlmodule-ubuntu-components0-ubuntu-components-0-1.html

5) The same for the pages for topics such as theming: there are no breadcrumbs there and one cannot go back to the home page [1]

  http://people.canonical.com/~dpm/sdk-docs/html/theming-components.html

6) Just a nitpick, but if technically possible, 'ubuntu' should be capitalized as 'Ubuntu' on the 'Modules ubuntu' line at the top of the home page [1].

[1] http://people.canonical.com/~dpm/sdk-docs/html/ubuntu-module.html

review: Needs Fixing

« Back to merge proposal