| Message ID | 1440440620-25937-52-git-send-email-damien.lespiau@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
> +table#patchlist > tbody > tr > td.tag-A { > + text-align: center; > + background-color: #bdecb6; > +} > + > +table#patchlist > tbody > tr > td.tag-R { > + text-align: center; > + background-color: #c3fdb8; > +} > + > +table#patchlist > tbody > tr > td.tag-T { > + text-align: center; > + background-color: #b4cfec; > +} This assumes you only have Acked/Reviewed/Tested-by tags. However, it is possible to add additional tags as required so this won't scale. Maybe it's not an issue? In any case, move 'text-align' into 'table#patchlist > tbody > tr' - no point in duplicating it.
On Tue, Aug 25, 2015 at 10:30:32AM +0100, Finucane, Stephen wrote: > > +table#patchlist > tbody > tr > td.tag-A { > > + text-align: center; > > + background-color: #bdecb6; > > +} > > + > > +table#patchlist > tbody > tr > td.tag-R { > > + text-align: center; > > + background-color: #c3fdb8; > > +} > > + > > +table#patchlist > tbody > tr > td.tag-T { > > + text-align: center; > > + background-color: #b4cfec; > > +} > > This assumes you only have Acked/Reviewed/Tested-by tags. However, it > is possible to add additional tags as required so this won't scale. > Maybe it's not an issue? Well, there's a lot to talk about with tags. Associating a color with the tag (in the db then) makes some sense. That's something that can be changed in the future though and I didn't want to introduce db changes before we have sorted out the pending patches touching the db. One note is that "genericity" is already biting us a bit in the patch state, it's a fine idea, but in practice all known patchwork instances seem to work with the same list of states, so all it does is introducing more complexity from my point of view. For tags though, I thing the idea can be extended quite a bit, but that's a whole different story. > In any case, move 'text-align' into 'table#patchlist > tbody > tr' - > no point in duplicating it. I don't believe so, I don't want to center all the <td> in that table, just the ones with those clases.
> Well, there's a lot to talk about with tags. Associating a color with > the tag (in the db then) makes some sense. That's something that can be > changed in the future though and I didn't want to introduce db changes > before we have sorted out the pending patches touching the db. Do it dynamically? > One note is that "genericity" is already biting us a bit in the patch > state, it's a fine idea, but in practice all known patchwork instances > seem to work with the same list of states, so all it does is introducing > more complexity from my point of view. Agreed. We should use 'choices'! :) > For tags though, I thing the idea can be extended quite a bit, but > that's a whole different story. > > > In any case, move 'text-align' into 'table#patchlist > tbody > tr' - > > no point in duplicating it. > > I don't believe so, I don't want to center all the <td> in that table, > just the ones with those clases. So add a generic class? Duplication is the root of all evil. > -- > Damien
diff --git a/htdocs/css/style.css b/htdocs/css/style.css index 96575df..8f56dc1 100644 --- a/htdocs/css/style.css +++ b/htdocs/css/style.css @@ -136,6 +136,21 @@ table#patchlist > thead { background-color: white; } +table#patchlist > tbody > tr > td.tag-A { + text-align: center; + background-color: #bdecb6; +} + +table#patchlist > tbody > tr > td.tag-R { + text-align: center; + background-color: #c3fdb8; +} + +table#patchlist > tbody > tr > td.tag-T { + text-align: center; + background-color: #b4cfec; +} + a.colinactive, a.colactive { color: black; text-decoration: none; diff --git a/patchwork/templates/patchwork/patch-list.html b/patchwork/templates/patchwork/patch-list.html index c81fe88..ce60a38 100644 --- a/patchwork/templates/patchwork/patch-list.html +++ b/patchwork/templates/patchwork/patch-list.html @@ -69,11 +69,11 @@ $(document).ready(function() { {% endifequal %} </th> +{% for tag in project.tags %} <th> - <span - title="{% for tag in project.tags %}{{tag.name}}{% if not forloop.last %} / {% endif %}{% endfor %}" - >{% for tag in project.tags %}{{tag.abbrev}}{% if not forloop.last %}/{% endif %}{% endfor %}</span> + <span title="{{tag.name}}">{{tag.abbrev}}</span> </th> +{% endfor %} <th> {% ifequal order.name "date" %} @@ -153,7 +153,9 @@ $(document).ready(function() { {% endif %} <td><a href="{% url 'patchwork.views.patch.patch' patch_id=patch.id %}" >{{ patch.name|default:"[no subject]"|truncatechars:100 }}</a></td> - <td style="white-space: nowrap;">{{ patch|patch_tags }}</td> +{% for tag in project.tags %} + {{ patch|patch_tags:tag }} +{% endfor %} <td>{{ patch.date|date:"Y-m-d" }}</td> <td>{{ patch.submitter|personify:project }}</td> <td>{{ patch.delegate.username }}</td> diff --git a/patchwork/templatetags/patch.py b/patchwork/templatetags/patch.py index ea23ebd..43da69d 100644 --- a/patchwork/templatetags/patch.py +++ b/patchwork/templatetags/patch.py @@ -66,13 +66,10 @@ class EditablePatchNode(template.Node): return self.nodelist_true.render(context) @register.filter(name='patch_tags') -def patch_tags(patch): - counts = [] - titles = [] - for tag in patch.project.tags: - count = getattr(patch, tag.attr_name) - titles.append('%d %s' % (count, tag.name)) - counts.append(str(count)) - return mark_safe('<span title="%s">%s</span>' % ( - ' / '.join(titles), - ' '.join(counts))) +def patch_tags(patch, tag): + count = getattr(patch, tag.attr_name) + count_str = str(count) if count else '' + class_str = "tag-%s" % tag.abbrev if count else '' + title = '%d %s' % (count, tag.name) + return mark_safe('<td class="%s"><span title="%s">%s</span></td>' % + (class_str, title, count_str))
- Use colors to distinguish the tags - Don't display any information when there's none (ie don't display "there's 0 r-b tag"). That part is something similar to what Thomas Petazzoni also did. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- htdocs/css/style.css | 15 +++++++++++++++ patchwork/templates/patchwork/patch-list.html | 10 ++++++---- patchwork/templatetags/patch.py | 17 +++++++---------- 3 files changed, 28 insertions(+), 14 deletions(-)