On 12/24/2015 09:32 PM, Jan Heylen wrote:
# HG changeset patch
# User Jan Heylen <[email protected]>
# Date 1448811822 -3600
# Sun Nov 29 16:43:42 2015 +0100
# Node ID 3bd79591517080ccf795e559650db057112b7219
# Parent f306d394f56a447d11521596df6ed730fbca9709
diff: diff_block_simple: make it collapsable and default behaviour selectable
Ok, thanks. Now I can try it out and understand. I was missing that it
was about making "diff_block_simple" (used in PRs and compare) behave
more like "diff_block" (used in changeset and file diffs) and giving it
the same collapse/expand options. (Please add something like that to the
commit message.)
But while diff_block has one Collapse link for the diff of all involved
files, this patch will give one link for each file (similar to the patch
from us I linked to), and after collapsing one file, the expand text
will be shadowed by the next collapse link. It has to work better with
multiple files.
I am all for making these diff views more consistent and having he same
amount of collapse/expand everywhere. I assume the different behaviour
was unintentional and caused by confusing difference in the two very
similar diff templates? It seems like the collapse link and container
should live in pullrequest_show.html instead ... but that would also be
weird. That very much suggests that it would be very nice to unify these
two diff templates ...
(Btw: I wonder what the use case is for hiding all diffs in a changeset
diff. Is it just to make it easy to scroll to the bottom of the page? (I
can see more of a point about collapsing it on PRs if you want to
encourage per changeset reviews. That wouldn't work for us right now ...
but that is a different story.))
/Mads
diff -r f306d394f56a -r 3bd795915170
kallithea/templates/changeset/diff_block.html
--- a/kallithea/templates/changeset/diff_block.html Thu Dec 24 21:28:19
2015 +0100
+++ b/kallithea/templates/changeset/diff_block.html Sun Nov 29 16:43:42
2015 +0100
@@ -65,8 +65,21 @@
</div>
</%def>
-<%def name="diff_block_simple(change)">
-
+<%def name="diff_block_simple(change, hidden=False)">
+<%
+if hidden:
+ container_class = "hidden"
+else:
+ container_class = ""
+%>
+<div class="diff-collapse">
+ %if hidden:
+ <span target="${'diff-container-%s' % (id(change))}"
class="diff-collapse-button">↓ ${_('Expand Diff')} ↓</span>
+ %else:
+ <span target="${'diff-container-%s' % (id(change))}"
class="diff-collapse-button">↑ ${_('Collapse Diff')} ↑</span>
+ %endif
+</div>
+<div class="diff-container ${container_class}" id="${'diff-container-%s' %
(id(change))}">
%for op,filenode_path,diff in change:
<div id="${h.FID('',filenode_path)}_target"
style="clear:both;margin-top:25px"></div>
<div id="${h.FID('',filenode_path)}" class="diffblock margined comm">
@@ -124,6 +137,7 @@
</div>
</div>
%endfor
+</div>
</%def>
<%def name="diff_block_js()">
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
