Code review comment for lp:~linaro-infrastructure/launchpad-work-items-tracker/add-lane-progress-bar

Revision history for this message
Guilherme Salgado (salgado) wrote :

I'm no CSS expert either, but it looked fine to me. I have just one
suggestion below:

On 13/01/12 12:03, Mattias Backman wrote:
> === modified file 'templates/roadmap_card.html'
> --- templates/roadmap_card.html 2011-12-21 14:05:45 +0000
> +++ templates/roadmap_card.html 2012-01-13 15:02:25 +0000
> @@ -35,7 +35,16 @@
> </ul>
>
> <div style="clear:both; text-align: center">Overall blueprint completion</div>
> -${util.progress_bar(blueprint_status_count) if card_has_blueprints else '<center><i>Progress graph pending linked blueprints.</i></center>'}
> +% if card_has_blueprints:
> +<div class="roadmap_wrap" title="${bp_status_totals['Completed']} blueprints complete of ${bp_status_totals['Total']}">
> + <div class="roadmap_value" style="width:${bp_status_totals['Percentage']}%">
> + <div class="Completed">&nbsp;</div>
> + </div>
> + <div class="roadmap_progress_text">${bp_status_totals['Percentage']} % complete of ${bp_status_totals['Total']}</div>
> +</div>

Would it be possible to move this into a function like
util.progress_bar() to avoid duplicating it in roadmap_lane.html?

> +% else:
> +<center><i>Progress graph pending linked blueprints.</i></center>
> +% endif
>
> <h3>Description</h3> ${card.description if card.description is not None else '<i>No description could be found.</i>'}
> <p><a href="${card.url}">Read the full description</a>.
>
> === modified file 'templates/roadmap_lane.html'
> --- templates/roadmap_lane.html 2011-12-22 11:57:53 +0000
> +++ templates/roadmap_lane.html 2012-01-13 15:02:25 +0000
> @@ -17,6 +17,12 @@
> % endfor
> </select>
> <h1>${title()}</h1>
> +<div class="roadmap_wrap" title="${bp_status_totals['Completed']} blueprints complete of ${bp_status_totals['Total']}">
> + <div class="roadmap_value" style="width:${bp_status_totals['Percentage']}%">
> + <div class="Completed">&nbsp;</div>
> + </div>
> + <div class="roadmap_progress_text">${bp_status_totals['Percentage']} % complete of ${bp_status_totals['Total']}</div>
> +</div>
> <p>
> <table width="100%">
> <thead><tr><th>Card</th><th>Status</th><th>Team</th><th>Priority</th><th>Blueprints</th><th>Health</th></tr></thead>
>

« Back to merge proposal