On 01/05/2016 08:36 PM, Jan Heylen wrote:
--- a/kallithea/templates/changeset/diff_block.html Sat Jan 02
11:53:38 2016 +0100
+++ b/kallithea/templates/changeset/diff_block.html Sat Jan 02
11:54:11 2016 +0100
@@ -31,6 +31,12 @@
<span target="${'diff-container-%s' % (id(change))}"
class="diff-collapse-button">↑ ${_('Collapse Diff')} ↑</span>
</div>
<div class="diff-container" id="${'diff-container-%s' % (id(change))}">
+ <span style="float:right;margin-bottom:3px;margin-right:30px">
+ <label>
+ ${_('Show inline comments')}
+
${h.checkbox('',checked="checked",class_="show-inline-comments",id_for="diff-container-%s"
% (id(change)))}
+ </label>
+ </span>
%for FID,(cs1, cs2, change, path, diff, stats) in change.iteritems():
<div id="${FID}_target" style="clear:both;margin-top:25px"></div>
<div id="${FID}" class="diffblock margined comm">
id() will essentially just take the memory address of the variable. That is
not good to use here. Other objects might get the same address when this
goes out of scope. Actually, it seems like you in this case only need a
globally unique identifier?
Indeed, I thought I get a unique identifier by "id(change)", but I'm
not sure what to use in replacement?
What should it be unique among? How many of these are there and what do
they represent?
I also see more use case for collapsing the individual file diffs (after
they have been reviewed so the reviewer can focus on the rest) than for
collapsing everything (leaving almost nothing on the page, only to guide the
reader somewhere else).
I agree, but the current collapse/expand code isn't functional for
this at this moment, maybe we can change the div box css class
changeset-header or code-header to be collapseble or expandable, with
the use of an existing javascript thing. Would it be feasible to
include jqueryui? We could use https://jqueryui.com/accordion
Yes, we could use more fancy UI. But we should also make sure the UI
doesn't get more inconsistent. A mix of different effects and styles is
not pleasant to use. For example, I find it annoying that our file
navigation is "application in the browser" with fake URLs while the rest
of Kallithea is page based with quite reasonable URLs and predictable
navigation.
/Mads
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general